From ac06f178fe716cbfd165169e04f806657e332be4 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Wed, 11 Dec 2024 14:22:09 +0100 Subject: [PATCH 1/5] Add policy rule source and destination resource Signed-off-by: bcmmbaga --- management/server/http/api/types.gen.go | 43 ++++++++++++++----- .../networks/resources/types/resource.go | 5 +++ management/server/types/policyrule.go | 10 +++++ 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/management/server/http/api/types.gen.go b/management/server/http/api/types.gen.go index 835a6ccff36..7e68268724d 100644 --- a/management/server/http/api/types.gen.go +++ b/management/server/http/api/types.gen.go @@ -143,6 +143,13 @@ const ( PolicyRuleUpdateProtocolUdp PolicyRuleUpdateProtocol = "udp" ) +// Defines values for ResourceType. +const ( + ResourceTypeDomain ResourceType = "domain" + ResourceTypeHost ResourceType = "host" + ResourceTypeSubnet ResourceType = "subnet" +) + // Defines values for UserStatus. const ( UserStatusActive UserStatus = "active" @@ -540,9 +547,6 @@ type NetworkResource struct { Type NetworkResourceType `json:"type"` } -// NetworkResourceType Network resource type based of the address -type NetworkResourceType string - // NetworkResourceRequest defines model for NetworkResourceRequest. type NetworkResourceRequest struct { // Address Network resource address (either a direct host like 1.1.1.1 or 1.1.1.1/32, or a subnet like 192.168.178.0/24, or a domain like example.com) @@ -555,6 +559,9 @@ type NetworkResourceRequest struct { Name string `json:"name"` } +// NetworkResourceType Network resource type based of the address +type NetworkResourceType string + // NetworkRouter defines model for NetworkRouter. type NetworkRouter struct { // Id Network Router Id @@ -873,10 +880,11 @@ type PolicyRule struct { Bidirectional bool `json:"bidirectional"` // Description Policy rule friendly description - Description *string `json:"description,omitempty"` + Description *string `json:"description,omitempty"` + DestinationResource *Resource `json:"destinationResource,omitempty"` // Destinations Policy rule destination group IDs - Destinations []GroupMinimum `json:"destinations"` + Destinations *[]GroupMinimum `json:"destinations,omitempty"` // Enabled Policy rule status Enabled bool `json:"enabled"` @@ -894,10 +902,11 @@ type PolicyRule struct { Ports *[]string `json:"ports,omitempty"` // Protocol Policy rule type of the traffic - Protocol PolicyRuleProtocol `json:"protocol"` + Protocol PolicyRuleProtocol `json:"protocol"` + SourceResource *Resource `json:"sourceResource,omitempty"` // Sources Policy rule source group IDs - Sources []GroupMinimum `json:"sources"` + Sources *[]GroupMinimum `json:"sources,omitempty"` } // PolicyRuleAction Policy rule accept or drops packets @@ -951,10 +960,11 @@ type PolicyRuleUpdate struct { Bidirectional bool `json:"bidirectional"` // Description Policy rule friendly description - Description *string `json:"description,omitempty"` + Description *string `json:"description,omitempty"` + DestinationResource *Resource `json:"destinationResource,omitempty"` // Destinations Policy rule destination group IDs - Destinations []string `json:"destinations"` + Destinations *[]string `json:"destinations,omitempty"` // Enabled Policy rule status Enabled bool `json:"enabled"` @@ -972,10 +982,11 @@ type PolicyRuleUpdate struct { Ports *[]string `json:"ports,omitempty"` // Protocol Policy rule type of the traffic - Protocol PolicyRuleUpdateProtocol `json:"protocol"` + Protocol PolicyRuleUpdateProtocol `json:"protocol"` + SourceResource *Resource `json:"sourceResource,omitempty"` // Sources Policy rule source group IDs - Sources []string `json:"sources"` + Sources *[]string `json:"sources,omitempty"` } // PolicyRuleUpdateAction Policy rule accept or drops packets @@ -1049,6 +1060,16 @@ type ProcessCheck struct { Processes []Process `json:"processes"` } +// Resource defines model for Resource. +type Resource struct { + // Id Resource ID + Id string `json:"id"` + Type ResourceType `json:"type"` +} + +// ResourceType defines model for ResourceType. +type ResourceType string + // Route defines model for Route. type Route struct { // AccessControlGroups Access control group identifier associated with route. diff --git a/management/server/networks/resources/types/resource.go b/management/server/networks/resources/types/resource.go index dd2bdd6b7d8..117561bb251 100644 --- a/management/server/networks/resources/types/resource.go +++ b/management/server/networks/resources/types/resource.go @@ -12,6 +12,11 @@ import ( "github.com/netbirdio/netbird/management/server/http/api" ) +type Resource struct { + ID string + Type string +} + type NetworkResourceType string const ( diff --git a/management/server/types/policyrule.go b/management/server/types/policyrule.go index b1a7ac9dc91..250d7011c47 100644 --- a/management/server/types/policyrule.go +++ b/management/server/types/policyrule.go @@ -1,5 +1,9 @@ package types +import ( + "github.com/netbirdio/netbird/management/server/networks/resources/types" +) + // PolicyUpdateOperationType operation type type PolicyUpdateOperationType int @@ -41,9 +45,15 @@ type PolicyRule struct { // Destinations policy destination groups Destinations []string `gorm:"serializer:json"` + // DestinationResource policy destination resource that the rule is applied to + DestinationResource types.Resource `gorm:"serializer:json"` + // Sources policy source groups Sources []string `gorm:"serializer:json"` + // SourceResource policy source resource that the rule is applied to + SourceResource types.Resource `gorm:"serializer:json"` + // Bidirectional define if the rule is applicable in both directions, sources, and destinations Bidirectional bool From 9fae103370ce343d2a9cf37a3c52c2382e0288c3 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Wed, 11 Dec 2024 14:22:33 +0100 Subject: [PATCH 2/5] Extends policy rule API with source and destination resource Signed-off-by: bcmmbaga --- management/server/http/api/openapi.yml | 43 ++++++++++--- .../handlers/policies/policies_handler.go | 60 +++++++++++++++++-- 2 files changed, 90 insertions(+), 13 deletions(-) diff --git a/management/server/http/api/openapi.yml b/management/server/http/api/openapi.yml index 98b2c7a89e1..65fbf585592 100644 --- a/management/server/http/api/openapi.yml +++ b/management/server/http/api/openapi.yml @@ -782,15 +782,18 @@ components: items: type: string example: "ch8i4ug6lnn4g9hqv797" + sourceResource: + description: Policy rule source resource that the rule is applied to + $ref: '#/components/schemas/Resource' destinations: description: Policy rule destination group IDs type: array items: type: string example: "ch8i4ug6lnn4g9h7v7m0" - required: - - sources - - destinations + destinationResource: + description: Policy rule destination resource that the rule is applied to + $ref: '#/components/schemas/Resource' PolicyRule: allOf: - $ref: '#/components/schemas/PolicyRuleMinimum' @@ -801,14 +804,17 @@ components: type: array items: $ref: '#/components/schemas/GroupMinimum' + sourceResource: + description: Policy rule source resource that the rule is applied to + $ref: '#/components/schemas/Resource' destinations: description: Policy rule destination group IDs type: array items: $ref: '#/components/schemas/GroupMinimum' - required: - - sources - - destinations + destinationResource: + description: Policy rule destination resource that the rule is applied to + $ref: '#/components/schemas/Resource' PolicyMinimum: type: object properties: @@ -1176,6 +1182,24 @@ components: - id - network_type - $ref: '#/components/schemas/RouteRequest' + Resource: + type: object + properties: + id: + description: Resource ID + type: string + example: chacdk86lnnboviihd7g + type: + description: Resource type + $ref: '#/components/schemas/ResourceType' + required: + - id + - type + ResourceType: + allOf: + - $ref: '#/components/schemas/NetworkResourceType' + - type: string + example: host NetworkRequest: type: object properties: @@ -1228,13 +1252,16 @@ components: example: chacdk86lnnboviihd7g type: description: Network resource type based of the address - type: string - enum: [ "host", "subnet", "domain"] + $ref: '#/components/schemas/NetworkResourceType' example: host required: - id - type - $ref: '#/components/schemas/NetworkResourceRequest' + NetworkResourceType: + description: Network resource type based of the address + type: string + enum: [ "host", "subnet", "domain" ] NetworkRouterRequest: type: object properties: diff --git a/management/server/http/handlers/policies/policies_handler.go b/management/server/http/handlers/policies/policies_handler.go index 9cae8f8232b..efa1142b406 100644 --- a/management/server/http/handlers/policies/policies_handler.go +++ b/management/server/http/handlers/policies/policies_handler.go @@ -14,6 +14,7 @@ import ( "github.com/netbirdio/netbird/management/server/http/configs" "github.com/netbirdio/netbird/management/server/http/util" "github.com/netbirdio/netbird/management/server/jwtclaims" + networkTypes "github.com/netbirdio/netbird/management/server/networks/resources/types" "github.com/netbirdio/netbird/management/server/status" "github.com/netbirdio/netbird/management/server/types" ) @@ -147,15 +148,58 @@ func (h *handler) savePolicy(w http.ResponseWriter, r *http.Request, accountID s ruleID = *rule.Id } + hasSources := rule.Sources != nil + hasSourceResource := rule.SourceResource != nil + + hasDestinations := rule.Destinations != nil + hasDestinationResource := rule.DestinationResource != nil + + if hasSources && hasSourceResource { + util.WriteError(r.Context(), status.Errorf(status.InvalidArgument, "specify either sources or source resources, not both"), w) + return + } + + if hasDestinations && hasDestinationResource { + util.WriteError(r.Context(), status.Errorf(status.InvalidArgument, "specify either destinations or destination resources, not both"), w) + return + } + + if !(hasSources || hasSourceResource) || !(hasDestinations || hasDestinationResource) { + util.WriteError(r.Context(), status.Errorf(status.InvalidArgument, "specify either sources or source resources and destinations or destination resources"), w) + return + } + pr := types.PolicyRule{ ID: ruleID, PolicyID: policyID, Name: rule.Name, - Destinations: rule.Destinations, - Sources: rule.Sources, Bidirectional: rule.Bidirectional, } + if hasSources { + pr.Sources = *rule.Sources + } + + if hasSourceResource { + // TODO: validate the resource id and type + pr.SourceResource = networkTypes.Resource{ + ID: rule.SourceResource.Id, + Type: string(rule.SourceResource.Type), + } + } + + if hasDestinations { + pr.Destinations = *rule.Destinations + } + + if hasDestinationResource { + // TODO: validate the resource id and type + pr.DestinationResource = networkTypes.Resource{ + ID: rule.DestinationResource.Id, + Type: string(rule.DestinationResource.Type), + } + } + pr.Enabled = rule.Enabled if rule.Description != nil { pr.Description = *rule.Description @@ -363,26 +407,30 @@ func toPolicyResponse(groups []*nbgroup.Group, policy *types.Policy) *api.Policy rule.PortRanges = &portRanges } + var sources []api.GroupMinimum for _, gid := range r.Sources { _, ok := cache[gid] if ok { continue } + if group, ok := groupsMap[gid]; ok { minimum := api.GroupMinimum{ Id: group.ID, Name: group.Name, PeersCount: len(group.Peers), } - rule.Sources = append(rule.Sources, minimum) + sources = append(sources, minimum) cache[gid] = minimum } } + rule.Sources = &sources + var destinations []api.GroupMinimum for _, gid := range r.Destinations { cachedMinimum, ok := cache[gid] if ok { - rule.Destinations = append(rule.Destinations, cachedMinimum) + destinations = append(destinations, cachedMinimum) continue } if group, ok := groupsMap[gid]; ok { @@ -391,10 +439,12 @@ func toPolicyResponse(groups []*nbgroup.Group, policy *types.Policy) *api.Policy Name: group.Name, PeersCount: len(group.Peers), } - rule.Destinations = append(rule.Destinations, minimum) + destinations = append(destinations, minimum) cache[gid] = minimum } } + rule.Destinations = &destinations + ap.Rules = append(ap.Rules, rule) } return ap From bde334ecfceff7950d8dbf333e795dc9f22cc59e Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Wed, 11 Dec 2024 14:37:36 +0100 Subject: [PATCH 3/5] Refactor Signed-off-by: bcmmbaga --- management/server/http/api/openapi.yml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/management/server/http/api/openapi.yml b/management/server/http/api/openapi.yml index 65fbf585592..d9f0fb9dab5 100644 --- a/management/server/http/api/openapi.yml +++ b/management/server/http/api/openapi.yml @@ -1186,11 +1186,11 @@ components: type: object properties: id: - description: Resource ID + description: ID of the resource type: string example: chacdk86lnnboviihd7g type: - description: Resource type + description: Type of the resource $ref: '#/components/schemas/ResourceType' required: - id @@ -1251,9 +1251,7 @@ components: type: string example: chacdk86lnnboviihd7g type: - description: Network resource type based of the address $ref: '#/components/schemas/NetworkResourceType' - example: host required: - id - type @@ -1262,6 +1260,7 @@ components: description: Network resource type based of the address type: string enum: [ "host", "subnet", "domain" ] + example: host NetworkRouterRequest: type: object properties: From e5a6f9e965016aba35cad11d73f748ef873e08fd Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Wed, 11 Dec 2024 16:15:42 +0100 Subject: [PATCH 4/5] Fix tests Signed-off-by: bcmmbaga --- .../http/handlers/policies/policies_handler_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/management/server/http/handlers/policies/policies_handler_test.go b/management/server/http/handlers/policies/policies_handler_test.go index 96ccb8a47d6..d8db288d6b7 100644 --- a/management/server/http/handlers/policies/policies_handler_test.go +++ b/management/server/http/handlers/policies/policies_handler_test.go @@ -177,7 +177,9 @@ func TestPoliciesWritePolicy(t *testing.T) { "Description": "Description", "Protocol": "tcp", "Action": "accept", - "Bidirectional":true + "Bidirectional":true, + "Sources": ["F"], + "Destinations": ["G"] } ]}`)), expectedStatus: http.StatusOK, @@ -193,6 +195,8 @@ func TestPoliciesWritePolicy(t *testing.T) { Protocol: "tcp", Action: "accept", Bidirectional: true, + Sources: &[]api.GroupMinimum{{Id: "F"}}, + Destinations: &[]api.GroupMinimum{{Id: "G"}}, }, }, }, @@ -221,7 +225,9 @@ func TestPoliciesWritePolicy(t *testing.T) { "Description": "Description", "Protocol": "tcp", "Action": "accept", - "Bidirectional":true + "Bidirectional":true, + "Sources": ["F"], + "Destinations": ["F"] } ]}`)), expectedStatus: http.StatusOK, @@ -237,6 +243,8 @@ func TestPoliciesWritePolicy(t *testing.T) { Protocol: "tcp", Action: "accept", Bidirectional: true, + Sources: &[]api.GroupMinimum{{Id: "F"}}, + Destinations: &[]api.GroupMinimum{{Id: "F"}}, }, }, }, From 9322a92ee92d9124049ff36af89f27d42c509c96 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Wed, 11 Dec 2024 16:16:23 +0100 Subject: [PATCH 5/5] Refactor the resource and add api helper functions Signed-off-by: bcmmbaga --- .../handlers/policies/policies_handler.go | 31 +++++++++---------- .../networks/resources/types/resource.go | 5 --- management/server/types/policyrule.go | 8 ++--- management/server/types/resource.go | 30 ++++++++++++++++++ 4 files changed, 47 insertions(+), 27 deletions(-) create mode 100644 management/server/types/resource.go diff --git a/management/server/http/handlers/policies/policies_handler.go b/management/server/http/handlers/policies/policies_handler.go index efa1142b406..0255c773be4 100644 --- a/management/server/http/handlers/policies/policies_handler.go +++ b/management/server/http/handlers/policies/policies_handler.go @@ -14,7 +14,6 @@ import ( "github.com/netbirdio/netbird/management/server/http/configs" "github.com/netbirdio/netbird/management/server/http/util" "github.com/netbirdio/netbird/management/server/jwtclaims" - networkTypes "github.com/netbirdio/netbird/management/server/networks/resources/types" "github.com/netbirdio/netbird/management/server/status" "github.com/netbirdio/netbird/management/server/types" ) @@ -182,10 +181,9 @@ func (h *handler) savePolicy(w http.ResponseWriter, r *http.Request, accountID s if hasSourceResource { // TODO: validate the resource id and type - pr.SourceResource = networkTypes.Resource{ - ID: rule.SourceResource.Id, - Type: string(rule.SourceResource.Type), - } + sourceResource := &types.Resource{} + sourceResource.FromAPIRequest(rule.SourceResource) + pr.SourceResource = *sourceResource } if hasDestinations { @@ -194,10 +192,9 @@ func (h *handler) savePolicy(w http.ResponseWriter, r *http.Request, accountID s if hasDestinationResource { // TODO: validate the resource id and type - pr.DestinationResource = networkTypes.Resource{ - ID: rule.DestinationResource.Id, - Type: string(rule.DestinationResource.Type), - } + destinationResource := &types.Resource{} + destinationResource.FromAPIRequest(rule.DestinationResource) + pr.DestinationResource = *destinationResource } pr.Enabled = rule.Enabled @@ -382,13 +379,15 @@ func toPolicyResponse(groups []*nbgroup.Group, policy *types.Policy) *api.Policy rID := r.ID rDescription := r.Description rule := api.PolicyRule{ - Id: &rID, - Name: r.Name, - Enabled: r.Enabled, - Description: &rDescription, - Bidirectional: r.Bidirectional, - Protocol: api.PolicyRuleProtocol(r.Protocol), - Action: api.PolicyRuleAction(r.Action), + Id: &rID, + Name: r.Name, + Enabled: r.Enabled, + Description: &rDescription, + Bidirectional: r.Bidirectional, + Protocol: api.PolicyRuleProtocol(r.Protocol), + Action: api.PolicyRuleAction(r.Action), + SourceResource: r.SourceResource.ToAPIResponse(), + DestinationResource: r.DestinationResource.ToAPIResponse(), } if len(r.Ports) != 0 { diff --git a/management/server/networks/resources/types/resource.go b/management/server/networks/resources/types/resource.go index 117561bb251..dd2bdd6b7d8 100644 --- a/management/server/networks/resources/types/resource.go +++ b/management/server/networks/resources/types/resource.go @@ -12,11 +12,6 @@ import ( "github.com/netbirdio/netbird/management/server/http/api" ) -type Resource struct { - ID string - Type string -} - type NetworkResourceType string const ( diff --git a/management/server/types/policyrule.go b/management/server/types/policyrule.go index 250d7011c47..bd9a9929202 100644 --- a/management/server/types/policyrule.go +++ b/management/server/types/policyrule.go @@ -1,9 +1,5 @@ package types -import ( - "github.com/netbirdio/netbird/management/server/networks/resources/types" -) - // PolicyUpdateOperationType operation type type PolicyUpdateOperationType int @@ -46,13 +42,13 @@ type PolicyRule struct { Destinations []string `gorm:"serializer:json"` // DestinationResource policy destination resource that the rule is applied to - DestinationResource types.Resource `gorm:"serializer:json"` + DestinationResource Resource `gorm:"serializer:json"` // Sources policy source groups Sources []string `gorm:"serializer:json"` // SourceResource policy source resource that the rule is applied to - SourceResource types.Resource `gorm:"serializer:json"` + SourceResource Resource `gorm:"serializer:json"` // Bidirectional define if the rule is applicable in both directions, sources, and destinations Bidirectional bool diff --git a/management/server/types/resource.go b/management/server/types/resource.go new file mode 100644 index 00000000000..820872f2004 --- /dev/null +++ b/management/server/types/resource.go @@ -0,0 +1,30 @@ +package types + +import ( + "github.com/netbirdio/netbird/management/server/http/api" +) + +type Resource struct { + ID string + Type string +} + +func (r *Resource) ToAPIResponse() *api.Resource { + if r.ID == "" && r.Type == "" { + return nil + } + + return &api.Resource{ + Id: r.ID, + Type: api.ResourceType(r.Type), + } +} + +func (r *Resource) FromAPIRequest(req *api.Resource) { + if req == nil { + return + } + + r.ID = req.Id + r.Type = string(req.Type) +}