From 7d0b10f789eed958eec8a91cdcf5359f18e34d70 Mon Sep 17 00:00:00 2001 From: Shyam Radhakrishnan Date: Tue, 22 Aug 2023 11:09:54 +0530 Subject: [PATCH 1/3] Add support for NSG chaining --- api/v1beta2/ocimanagedcluster_webhook.go | 10 +- api/v1beta2/types.go | 10 +- cloud/scope/nsg_reconciler.go | 70 ++++++--- cloud/scope/nsg_reconciler_test.go | 137 +++++++++++++++--- .../cluster.yaml | 4 +- 5 files changed, 181 insertions(+), 50 deletions(-) diff --git a/api/v1beta2/ocimanagedcluster_webhook.go b/api/v1beta2/ocimanagedcluster_webhook.go index c16ffe20..e0440261 100644 --- a/api/v1beta2/ocimanagedcluster_webhook.go +++ b/api/v1beta2/ocimanagedcluster_webhook.go @@ -311,7 +311,7 @@ func (c *OCIManagedCluster) GetControlPlaneEndpointDefaultEgressRules() []Egress EgressSecurityRule: EgressSecurityRule{ Description: common.String("Allow Kubernetes API endpoint to communicate with OKE."), Protocol: common.String("6"), - DestinationType: EgressSecurityRuleSourceTypeServiceCidrBlock, + DestinationType: EgressSecurityRuleDestinationTypeServiceCidrBlock, }, }, { @@ -322,7 +322,7 @@ func (c *OCIManagedCluster) GetControlPlaneEndpointDefaultEgressRules() []Egress Type: common.Int(3), Code: common.Int(4), }, - DestinationType: EgressSecurityRuleSourceTypeServiceCidrBlock, + DestinationType: EgressSecurityRuleDestinationTypeServiceCidrBlock, }, }, { @@ -413,7 +413,7 @@ func (c *OCIManagedCluster) GetWorkerDefaultEgressRules() []EgressSecurityRuleFo EgressSecurityRule: EgressSecurityRule{ Description: common.String("Allow worker nodes to communicate with OKE."), Protocol: common.String("6"), - DestinationType: EgressSecurityRuleSourceTypeServiceCidrBlock, + DestinationType: EgressSecurityRuleDestinationTypeServiceCidrBlock, }, }, { @@ -502,7 +502,7 @@ func (c *OCIManagedCluster) GetPodDefaultEgressRules() []EgressSecurityRuleForNS EgressSecurityRule: EgressSecurityRule{ Description: common.String("Allow worker nodes to communicate with OCI Services."), Protocol: common.String("6"), - DestinationType: EgressSecurityRuleSourceTypeServiceCidrBlock, + DestinationType: EgressSecurityRuleDestinationTypeServiceCidrBlock, }, }, { @@ -513,7 +513,7 @@ func (c *OCIManagedCluster) GetPodDefaultEgressRules() []EgressSecurityRuleForNS Type: common.Int(3), Code: common.Int(4), }, - DestinationType: EgressSecurityRuleSourceTypeServiceCidrBlock, + DestinationType: EgressSecurityRuleDestinationTypeServiceCidrBlock, }, }, { diff --git a/api/v1beta2/types.go b/api/v1beta2/types.go index f7be929b..a282d033 100644 --- a/api/v1beta2/types.go +++ b/api/v1beta2/types.go @@ -673,6 +673,8 @@ type EgressSecurityRule struct { // * `SERVICE_CIDR_BLOCK`: If the rule's `destination` is the `cidrBlock` value for a // Service (the rule is for traffic destined for a // particular `Service` through a service gateway). + // * `NETWORK_SECURITY_GROUP`: If the rule's `destination` is the OCID (https://docs.cloud.oracle.com/iaas/Content/General/Concepts/identifiers.htm) of a + // NetworkSecurityGroup. DestinationType EgressSecurityRuleDestinationTypeEnum `json:"destinationType,omitempty"` IcmpOptions *IcmpOptions `json:"icmpOptions,omitempty"` @@ -726,6 +728,8 @@ type IngressSecurityRule struct { // * `SERVICE_CIDR_BLOCK`: If the rule's `source` is the `cidrBlock` value for a // Service (the rule is for traffic coming from a // particular `Service` through a service gateway). + // * `NETWORK_SECURITY_GROUP`: If the rule's `destination` is the OCID (https://docs.cloud.oracle.com/iaas/Content/General/Concepts/identifiers.htm) of a + // NetworkSecurityGroup. SourceType IngressSecurityRuleSourceTypeEnum `json:"sourceType,omitempty"` TcpOptions *TcpOptions `json:"tcpOptions,omitempty"` @@ -753,6 +757,7 @@ type IngressSecurityRuleSourceTypeEnum string const ( IngressSecurityRuleSourceTypeCidrBlock IngressSecurityRuleSourceTypeEnum = "CIDR_BLOCK" IngressSecurityRuleSourceTypeServiceCidrBlock IngressSecurityRuleSourceTypeEnum = "SERVICE_CIDR_BLOCK" + IngressSecurityRuleSourceTypeNSG IngressSecurityRuleSourceTypeEnum = "NETWORK_SECURITY_GROUP" ) // UdpOptions Optional and valid only for UDP. Use to specify particular destination ports for UDP rules. @@ -802,8 +807,9 @@ type PortRange struct { const ( // EgressSecurityRuleDestinationTypeCidrBlock is the contant for CIDR block security rule destination type - EgressSecurityRuleDestinationTypeCidrBlock EgressSecurityRuleDestinationTypeEnum = "CIDR_BLOCK" - EgressSecurityRuleSourceTypeServiceCidrBlock EgressSecurityRuleDestinationTypeEnum = "SERVICE_CIDR_BLOCK" + EgressSecurityRuleDestinationTypeCidrBlock EgressSecurityRuleDestinationTypeEnum = "CIDR_BLOCK" + EgressSecurityRuleDestinationTypeServiceCidrBlock EgressSecurityRuleDestinationTypeEnum = "SERVICE_CIDR_BLOCK" + EgressSecurityRuleDestinationTypeNSG EgressSecurityRuleDestinationTypeEnum = "NETWORK_SECURITY_GROUP" ) type EgressSecurityRuleDestinationTypeEnum string diff --git a/cloud/scope/nsg_reconciler.go b/cloud/scope/nsg_reconciler.go index 66cf26ad..6594c131 100644 --- a/cloud/scope/nsg_reconciler.go +++ b/cloud/scope/nsg_reconciler.go @@ -47,14 +47,6 @@ func (s *ClusterScope) ReconcileNSG(ctx context.Context) error { } s.Logger.Info("Successfully updated network security list", "nsg", nsgOCID) } - s.adjustNSGRulesSpec(desiredNSG) - isNSGUpdated, err := s.UpdateNSGSecurityRulesIfNeeded(ctx, *desiredNSG, nsg) - if err != nil { - return err - } - if !isNSGUpdated { - s.Logger.Info("No Reconciliation Required for Network Security Group", "nsg", *desiredNSG.ID) - } continue } s.Logger.Info("Creating the network security list") @@ -64,16 +56,22 @@ func (s *ClusterScope) ReconcileNSG(ctx context.Context) error { } s.Logger.Info("Created the nsg", "nsg", nsgID) desiredNSG.ID = nsgID - s.adjustNSGRulesSpec(desiredNSG) - err = s.AddNSGSecurityRules(ctx, desiredNSG.ID, desiredNSG.IngressRules, desiredNSG.EgressRules) + + } + for _, desiredNSG := range desiredNSGs.List { + s.adjustNSGRulesSpec(desiredNSG, desiredNSGs.List) + isNSGUpdated, err := s.UpdateNSGSecurityRulesIfNeeded(ctx, *desiredNSG, desiredNSG.ID) if err != nil { return err } + if !isNSGUpdated { + s.Logger.Info("No Reconciliation Required for Network Security Group", "nsg", *desiredNSG.ID) + } } return nil } -func (s *ClusterScope) adjustNSGRulesSpec(desiredNSG *infrastructurev1beta2.NSG) { +func (s *ClusterScope) adjustNSGRulesSpec(desiredNSG *infrastructurev1beta2.NSG, nsgList []*infrastructurev1beta2.NSG) { ingressRules := make([]infrastructurev1beta2.IngressSecurityRuleForNSG, 0) for _, ingressRule := range desiredNSG.IngressRules { if ingressRule.SourceType == infrastructurev1beta2.IngressSecurityRuleSourceTypeServiceCidrBlock { @@ -84,7 +82,7 @@ func (s *ClusterScope) adjustNSGRulesSpec(desiredNSG *infrastructurev1beta2.NSG) desiredNSG.IngressRules = ingressRules egressRules := make([]infrastructurev1beta2.EgressSecurityRuleForNSG, 0) for _, egressRule := range desiredNSG.EgressRules { - if egressRule.DestinationType == infrastructurev1beta2.EgressSecurityRuleSourceTypeServiceCidrBlock { + if egressRule.DestinationType == infrastructurev1beta2.EgressSecurityRuleDestinationTypeServiceCidrBlock { egressRule.Destination = common.String(fmt.Sprintf("all-%s-services-in-oracle-services-network", strings.ToLower(s.RegionKey))) } egressRules = append(egressRules, egressRule) @@ -171,19 +169,19 @@ func (s *ClusterScope) IsNSGEqual(actual *core.NetworkSecurityGroup, desired inf // UpdateNSGSecurityRulesIfNeeded updates NSG rules if required by comparing actual and desired. func (s *ClusterScope) UpdateNSGSecurityRulesIfNeeded(ctx context.Context, desired infrastructurev1beta2.NSG, - actual *core.NetworkSecurityGroup) (bool, error) { + nsgId *string) (bool, error) { var ingressRulesToAdd []infrastructurev1beta2.IngressSecurityRuleForNSG var egressRulesToAdd []infrastructurev1beta2.EgressSecurityRuleForNSG var securityRulesToRemove []string var isNSGUpdated bool listSecurityRulesResponse, err := s.VCNClient.ListNetworkSecurityGroupSecurityRules(ctx, core.ListNetworkSecurityGroupSecurityRulesRequest{ - NetworkSecurityGroupId: actual.Id, + NetworkSecurityGroupId: nsgId, }) if err != nil { s.Logger.Error(err, "failed to reconcile the network security group, failed to list security rules") return isNSGUpdated, errors.Wrap(err, "failed to reconcile the network security group, failed to list security rules") } - ingressRules, egressRules := generateSpecFromSecurityRules(listSecurityRulesResponse.Items) + ingressRules, egressRules := s.generateSpecFromSecurityRules(listSecurityRulesResponse.Items, nsgId) for i, ingressRule := range desired.IngressRules { if ingressRule.IsStateless == nil { @@ -255,7 +253,7 @@ func (s *ClusterScope) UpdateNSGSecurityRulesIfNeeded(ctx context.Context, desir s.Logger.Error(err, "failed to reconcile the network security group, failed to add security rules") return isNSGUpdated, err } - s.Logger.Info("Successfully added missing rules in NSG", "nsg", *actual.Id) + s.Logger.Info("Successfully added missing rules in NSG", "nsg", *nsgId) } if len(securityRulesToRemove) > 0 { isNSGUpdated = true @@ -269,7 +267,7 @@ func (s *ClusterScope) UpdateNSGSecurityRulesIfNeeded(ctx context.Context, desir s.Logger.Error(err, "failed to reconcile the network security group, failed to remove security rules") return isNSGUpdated, err } - s.Logger.Info("Successfully deleted rules in NSG", "nsg", *actual.Id) + s.Logger.Info("Successfully deleted rules in NSG", "nsg", *nsgId) } return isNSGUpdated, nil } @@ -315,6 +313,9 @@ func (s *ClusterScope) generateAddSecurityRuleFromSpec(ingressRules []infrastruc TcpOptions: tcpOptions, UdpOptions: udpOptions, } + if ingressRule.SourceType == infrastructurev1beta2.IngressSecurityRuleSourceTypeNSG { + secRule.Source = getNsgIdFromName(ingressRule.Source, s.GetNSGSpec()) + } securityRules = append(securityRules, secRule) } for _, egressRule := range egressRules { @@ -335,15 +336,15 @@ func (s *ClusterScope) generateAddSecurityRuleFromSpec(ingressRules []infrastruc TcpOptions: tcpOptions, UdpOptions: udpOptions, } - securityRules = append(securityRules, secRule) - if egressRule.DestinationType == "SERVICE_CIDR_BLOCK" { - secRule.Destination = common.String(fmt.Sprintf("all-%s-services-in-oracle-services-network", strings.ToLower(s.RegionKey))) + if egressRule.DestinationType == infrastructurev1beta2.EgressSecurityRuleDestinationTypeNSG { + secRule.Destination = getNsgIdFromName(egressRule.Destination, s.GetNSGSpec()) } + securityRules = append(securityRules, secRule) } return securityRules } -func generateSpecFromSecurityRules(rules []core.SecurityRule) (map[string]infrastructurev1beta2.IngressSecurityRuleForNSG, map[string]infrastructurev1beta2.EgressSecurityRuleForNSG) { +func (s *ClusterScope) generateSpecFromSecurityRules(rules []core.SecurityRule, nsgId *string) (map[string]infrastructurev1beta2.IngressSecurityRuleForNSG, map[string]infrastructurev1beta2.EgressSecurityRuleForNSG) { var ingressRules = make(map[string]infrastructurev1beta2.IngressSecurityRuleForNSG) var egressRules = make(map[string]infrastructurev1beta2.EgressSecurityRuleForNSG) var stateless *bool @@ -368,6 +369,9 @@ func generateSpecFromSecurityRules(rules []core.SecurityRule) (map[string]infras Description: rule.Description, }, } + if rule.SourceType == core.SecurityRuleSourceTypeNetworkSecurityGroup { + ingressRule.IngressSecurityRule.Source = getNsgNameFromId(rule.Source, s.GetNSGSpec()) + } ingressRules[*rule.Id] = ingressRule case core.SecurityRuleDirectionEgress: egressRule := infrastructurev1beta2.EgressSecurityRuleForNSG{ @@ -382,6 +386,9 @@ func generateSpecFromSecurityRules(rules []core.SecurityRule) (map[string]infras Description: rule.Description, }, } + if rule.DestinationType == core.SecurityRuleDestinationTypeNetworkSecurityGroup { + egressRule.EgressSecurityRule.Destination = getNsgNameFromId(rule.Destination, s.GetNSGSpec()) + } egressRules[*rule.Id] = egressRule } } @@ -465,3 +472,24 @@ func getProtocolOptionsForSpec(icmp *core.IcmpOptions, tcp *core.TcpOptions, udp } return icmpOptions, tcpOptions, udpOptions } + +func getNsgIdFromName(nsgName *string, list []*infrastructurev1beta2.NSG) *string { + for _, nsg := range list { + if nsg.Name == *nsgName { + return nsg.ID + } + } + return nil +} + +func getNsgNameFromId(nsgId *string, list []*infrastructurev1beta2.NSG) *string { + if nsgId == nil { + return nil + } + for _, nsg := range list { + if reflect.DeepEqual(nsg.ID, nsgId) { + return &nsg.Name + } + } + return nil +} diff --git a/cloud/scope/nsg_reconciler_test.go b/cloud/scope/nsg_reconciler_test.go index 904cba48..42d556e5 100644 --- a/cloud/scope/nsg_reconciler_test.go +++ b/cloud/scope/nsg_reconciler_test.go @@ -241,6 +241,20 @@ func TestClusterScope_ReconcileNSG(t *testing.T) { Source: common.String("0.0.0.0/0"), }, }, + { + IngressSecurityRule: infrastructurev1beta2.IngressSecurityRule{ + Description: common.String("External access to Kubernetes API endpoint - 1"), + Protocol: common.String("6"), + TcpOptions: &infrastructurev1beta2.TcpOptions{ + DestinationPortRange: &infrastructurev1beta2.PortRange{ + Max: common.Int(6443), + Min: common.Int(6443), + }, + }, + SourceType: infrastructurev1beta2.IngressSecurityRuleSourceTypeNSG, + Source: common.String("no-update"), + }, + }, } customNSGEgress := []infrastructurev1beta2.EgressSecurityRuleForNSG{ { @@ -257,6 +271,20 @@ func TestClusterScope_ReconcileNSG(t *testing.T) { Destination: common.String(ControlPlaneMachineSubnetDefaultCIDR), }, }, + { + EgressSecurityRule: infrastructurev1beta2.EgressSecurityRule{ + Description: common.String("All traffic to control plane nodes - 1"), + Protocol: common.String("6"), + TcpOptions: &infrastructurev1beta2.TcpOptions{ + DestinationPortRange: &infrastructurev1beta2.PortRange{ + Max: common.Int(6443), + Min: common.Int(6443), + }, + }, + DestinationType: infrastructurev1beta2.EgressSecurityRuleDestinationTypeNSG, + Destination: common.String("update-rules"), + }, + }, } definedTagsInterface := make(map[string]map[string]interface{}) @@ -373,6 +401,21 @@ func TestClusterScope_ReconcileNSG(t *testing.T) { }, }, }, + { + Direction: core.SecurityRuleDirectionEgress, + Protocol: common.String("6"), + Description: common.String("All traffic to control plane nodes - 1"), + Destination: common.String("update-rules-id"), + DestinationType: core.SecurityRuleDestinationTypeNetworkSecurityGroup, + Id: common.String("egress-id-1"), + IsStateless: common.Bool(false), + TcpOptions: &core.TcpOptions{ + DestinationPortRange: &core.PortRange{ + Max: common.Int(6443), + Min: common.Int(6443), + }, + }, + }, { Direction: core.SecurityRuleDirectionIngress, Protocol: common.String("6"), @@ -388,6 +431,21 @@ func TestClusterScope_ReconcileNSG(t *testing.T) { }, }, }, + { + Direction: core.SecurityRuleDirectionIngress, + Protocol: common.String("6"), + Description: common.String("External access to Kubernetes API endpoint - 1"), + Id: common.String("ingress-id-1"), + Source: common.String("no-update-id"), + SourceType: core.SecurityRuleSourceTypeNetworkSecurityGroup, + IsStateless: common.Bool(false), + TcpOptions: &core.TcpOptions{ + DestinationPortRange: &core.PortRange{ + Max: common.Int(6443), + Min: common.Int(6443), + }, + }, + }, }, }, nil) @@ -410,6 +468,21 @@ func TestClusterScope_ReconcileNSG(t *testing.T) { }, }, }, + { + Direction: core.SecurityRuleDirectionEgress, + Protocol: common.String("6"), + Description: common.String("All traffic to control plane nodes - 1"), + Destination: common.String("update-rules-id"), + DestinationType: core.SecurityRuleDestinationTypeNetworkSecurityGroup, + Id: common.String("egress-id-1"), + IsStateless: common.Bool(false), + TcpOptions: &core.TcpOptions{ + DestinationPortRange: &core.PortRange{ + Max: common.Int(6443), + Min: common.Int(6443), + }, + }, + }, { Direction: core.SecurityRuleDirectionIngress, Protocol: common.String("6"), @@ -425,6 +498,21 @@ func TestClusterScope_ReconcileNSG(t *testing.T) { }, }, }, + { + Direction: core.SecurityRuleDirectionIngress, + Protocol: common.String("6"), + Description: common.String("External access to Kubernetes API endpoint - 1"), + Id: common.String("ingress-id-1"), + Source: common.String("no-update-id"), + SourceType: core.SecurityRuleSourceTypeNetworkSecurityGroup, + IsStateless: common.Bool(false), + TcpOptions: &core.TcpOptions{ + DestinationPortRange: &core.PortRange{ + Max: common.Int(6443), + Min: common.Int(6443), + }, + }, + }, }, }, nil) @@ -482,6 +570,20 @@ func TestClusterScope_ReconcileNSG(t *testing.T) { }, }, }, + { + Direction: core.AddSecurityRuleDetailsDirectionIngress, + Protocol: common.String("6"), + Description: common.String("External access to Kubernetes API endpoint - 1"), + Source: common.String("no-update-id"), + SourceType: core.AddSecurityRuleDetailsSourceTypeNetworkSecurityGroup, + IsStateless: common.Bool(false), + TcpOptions: &core.TcpOptions{ + DestinationPortRange: &core.PortRange{ + Max: common.Int(6443), + Min: common.Int(6443), + }, + }, + }, { Direction: core.AddSecurityRuleDetailsDirectionEgress, Protocol: common.String("6"), @@ -496,6 +598,20 @@ func TestClusterScope_ReconcileNSG(t *testing.T) { }, }, }, + { + Direction: core.AddSecurityRuleDetailsDirectionEgress, + Protocol: common.String("6"), + Description: common.String("All traffic to control plane nodes - 1"), + Destination: common.String("update-rules-id"), + DestinationType: core.AddSecurityRuleDetailsDestinationTypeNetworkSecurityGroup, + IsStateless: common.Bool(false), + TcpOptions: &core.TcpOptions{ + DestinationPortRange: &core.PortRange{ + Max: common.Int(6443), + Min: common.Int(6443), + }, + }, + }, }, }, })).Return(core.AddNetworkSecurityGroupSecurityRulesResponse{ @@ -706,26 +822,7 @@ func TestClusterScope_ReconcileNSG(t *testing.T) { })).Return(core.ListNetworkSecurityGroupSecurityRulesResponse{ Items: []core.SecurityRule{}, }, nil) - vcnClient.EXPECT().AddNetworkSecurityGroupSecurityRules(gomock.Any(), gomock.Eq(core.AddNetworkSecurityGroupSecurityRulesRequest{ - NetworkSecurityGroupId: common.String("loadbalancer-nsg-id"), - AddNetworkSecurityGroupSecurityRulesDetails: core.AddNetworkSecurityGroupSecurityRulesDetails{SecurityRules: []core.AddSecurityRuleDetails{ - { - Direction: core.AddSecurityRuleDetailsDirectionEgress, - Protocol: common.String("6"), - Description: common.String("All traffic to control plane nodes"), - Destination: common.String(ControlPlaneMachineSubnetDefaultCIDR), - DestinationType: core.AddSecurityRuleDetailsDestinationTypeCidrBlock, - IsStateless: common.Bool(false), - TcpOptions: &core.TcpOptions{ - DestinationPortRange: &core.PortRange{ - Max: common.Int(6443), - Min: common.Int(6443), - }, - }, - }, - }, - }, - })).Return(core.AddNetworkSecurityGroupSecurityRulesResponse{}, errors.New("some error")) + vcnClient.EXPECT().AddNetworkSecurityGroupSecurityRules(gomock.Any(), gomock.Any()).Return(core.AddNetworkSecurityGroupSecurityRulesResponse{}, errors.New("some error")) }, wantErr: true, expectedError: "failed add nsg security rules: some error", diff --git a/test/e2e/data/infrastructure-oci/v1beta1/cluster-template-custom-networking-nsg/cluster.yaml b/test/e2e/data/infrastructure-oci/v1beta1/cluster-template-custom-networking-nsg/cluster.yaml index 5b30316a..6ce38776 100644 --- a/test/e2e/data/infrastructure-oci/v1beta1/cluster-template-custom-networking-nsg/cluster.yaml +++ b/test/e2e/data/infrastructure-oci/v1beta1/cluster-template-custom-networking-nsg/cluster.yaml @@ -14,9 +14,9 @@ spec: egressRules: - egressRule: isStateless: false - destination: "15.0.5.0/28" + destination: "cp-mc-nsg" protocol: "6" - destinationType: "CIDR_BLOCK" + destinationType: "NETWORK_SECURITY_GROUP" description: "All traffic to control plane nodes" tcpOptions: destinationPortRange: From e45107ab62d9ce882750bc1bc6bf8cb26755c097 Mon Sep 17 00:00:00 2001 From: Shyam Radhakrishnan Date: Tue, 22 Aug 2023 12:54:21 +0530 Subject: [PATCH 2/3] Add support for NSG chaining --- docs/src/networking/custom-networking.md | 44 ++++++++++++++++++++++++ test/e2e/cluster_test.go | 22 ++++++++---- 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/docs/src/networking/custom-networking.md b/docs/src/networking/custom-networking.md index c8708ec6..83cceb74 100644 --- a/docs/src/networking/custom-networking.md +++ b/docs/src/networking/custom-networking.md @@ -339,6 +339,50 @@ spec: min: 6443 ``` +## Example spec to use Network Security Group as destination in security rule + +The spec below shows how to specify a Network Security Group as a destination in security rule. The Network Security +Group name is mentioned in the `destination` field in the below example. All the required Network Security Groups +must be defined in the template, CAPOCI will not lookup Network Security Group from the VCN. + +```yaml +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1beta2 +kind: OCICluster +metadata: + name: "${CLUSTER_NAME}" +spec: + compartmentId: "${OCI_COMPARTMENT_ID}" + networkSpec: + vcn: + name: ${CLUSTER_NAME} + cidr: "172.16.0.0/16" + networkSecurityGroup: + list: + - name: ep-nsg + role: control-plane-endpoint + egressRules: + - egressRule: + isStateless: false + destination: "cp-mc-nsg" + protocol: "6" + destinationType: "NETWORK_SECURITY_GROUP" + description: "All traffic to control plane nodes" + tcpOptions: + destinationPortRange: + max: 6443 + min: 6443 + - name: cp-mc-nsg + role: control-plane + egressRules: + - egressRule: + isStateless: false + destination: "0.0.0.0/0" + protocol: "6" + destinationType: "CIDR_BLOCK" + description: "control plane machine access to internet" +``` + [sl-vs-nsg]: https://docs.oracle.com/en-us/iaas/Content/Network/Concepts/securityrules.htm#comparison [externally-managed-cluster-infrastructure]: ../gs/externally-managed-cluster-infrastructure.md#example-spec-for-externally-managed-vcn-infrastructure [oci-nlb]: https://docs.oracle.com/en-us/iaas/Content/NetworkLoadBalancer/introducton.htm#Overview diff --git a/test/e2e/cluster_test.go b/test/e2e/cluster_test.go index 3bca8901..d1defc49 100644 --- a/test/e2e/cluster_test.go +++ b/test/e2e/cluster_test.go @@ -25,6 +25,7 @@ import ( "fmt" "os" "path/filepath" + "reflect" "strings" . "github.com/onsi/ginkgo/v2" @@ -734,7 +735,7 @@ func validateCustonNSGNetworking(ctx context.Context, ociCluster infrastructurev Expect(err).NotTo(HaveOccurred()) switch nsg.Role { case infrastructurev1beta1.ControlPlaneEndpointRole: - verifyNsg(ctx, "ep-nsg", resp, ociClusterOriginal, infrastructurev1beta1.ControlPlaneEndpointRole) + verifyNsg(ctx, resp, ociClusterOriginal, infrastructurev1beta1.ControlPlaneEndpointRole) lbId := ociCluster.Spec.NetworkSpec.APIServerLB.LoadBalancerId lb, err := lbClient.GetNetworkLoadBalancer(ctx, networkloadbalancer.GetNetworkLoadBalancerRequest{ NetworkLoadBalancerId: lbId, @@ -742,15 +743,15 @@ func validateCustonNSGNetworking(ctx context.Context, ociCluster infrastructurev Expect(err).NotTo(HaveOccurred()) Expect(lb.NetworkSecurityGroupIds[0]).To(Equal(*nsgId)) case infrastructurev1beta1.ControlPlaneRole: - verifyNsg(ctx, "cp-mc-nsg", resp, ociClusterOriginal, infrastructurev1beta1.ControlPlaneRole) + verifyNsg(ctx, resp, ociClusterOriginal, infrastructurev1beta1.ControlPlaneRole) Log("Validating control plane machine vnic NSG") validateVnicNSG(ctx, clusterName, nameSpace, nsgId, "") case infrastructurev1beta1.WorkerRole: - verifyNsg(ctx, "worker", resp, ociClusterOriginal, infrastructurev1beta1.WorkerRole) + verifyNsg(ctx, resp, ociClusterOriginal, infrastructurev1beta1.WorkerRole) Log("Validating node machine vnic NSG") validateVnicNSG(ctx, clusterName, nameSpace, nsgId, machineDeployment) case infrastructurev1beta1.ServiceLoadBalancerRole: - verifyNsg(ctx, "service-lb-nsg", resp, ociClusterOriginal, infrastructurev1beta1.ServiceLoadBalancerRole) + verifyNsg(ctx, resp, ociClusterOriginal, infrastructurev1beta1.ServiceLoadBalancerRole) default: return errors.New("invalid nsg role") } @@ -883,12 +884,12 @@ func verifySeclistSubnet(ctx context.Context, resp core.GetSubnetResponse, ociCl Expect(matches).To(Equal(1)) } -func verifyNsg(ctx context.Context, displayName string, resp core.GetNetworkSecurityGroupResponse, ociClusterOriginal *infrastructurev1beta1.OCICluster, role infrastructurev1beta1.Role) { +func verifyNsg(ctx context.Context, resp core.GetNetworkSecurityGroupResponse, ociClusterOriginal *infrastructurev1beta1.OCICluster, role infrastructurev1beta1.Role) { listResponse, err := vcnClient.ListNetworkSecurityGroupSecurityRules(ctx, core.ListNetworkSecurityGroupSecurityRulesRequest{ NetworkSecurityGroupId: resp.Id, }) Expect(err).NotTo(HaveOccurred()) - ingressRules, egressRules := generateSpecFromSecurityRules(listResponse.Items) + ingressRules, egressRules := generateSpecFromSecurityRules(ociClusterOriginal, listResponse.Items) matches := 0 for _, n := range ociClusterOriginal.Spec.NetworkSpec.Vcn.NetworkSecurityGroups { if n.Role == role { @@ -900,7 +901,7 @@ func verifyNsg(ctx context.Context, displayName string, resp core.GetNetworkSecu Expect(matches).To(Equal(1)) } -func generateSpecFromSecurityRules(rules []core.SecurityRule) ([]infrastructurev1beta1.IngressSecurityRuleForNSG, []infrastructurev1beta1.EgressSecurityRuleForNSG) { +func generateSpecFromSecurityRules(ociClusterOriginal *infrastructurev1beta1.OCICluster, rules []core.SecurityRule) ([]infrastructurev1beta1.IngressSecurityRuleForNSG, []infrastructurev1beta1.EgressSecurityRuleForNSG) { var ingressRules []infrastructurev1beta1.IngressSecurityRuleForNSG var egressRules []infrastructurev1beta1.EgressSecurityRuleForNSG for _, rule := range rules { @@ -938,6 +939,13 @@ func generateSpecFromSecurityRules(rules []core.SecurityRule) ([]infrastructurev Description: rule.Description, }, } + if rule.DestinationType == core.SecurityRuleDestinationTypeNetworkSecurityGroup { + for _, nsg := range ociClusterOriginal.Spec.NetworkSpec.Vcn.NetworkSecurityGroups { + if reflect.DeepEqual(nsg.ID, rule.Destination) { + egressRule.Destination = common.String(nsg.Name) + } + } + } egressRules = append(egressRules, egressRule) } } From 81e28d47a9239522af590992f7a946e78dc11ede Mon Sep 17 00:00:00 2001 From: Shyam Radhakrishnan Date: Tue, 22 Aug 2023 13:18:57 +0530 Subject: [PATCH 3/3] Add support for NSG chaining --- test/e2e/cluster_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/e2e/cluster_test.go b/test/e2e/cluster_test.go index d1defc49..a23a8e14 100644 --- a/test/e2e/cluster_test.go +++ b/test/e2e/cluster_test.go @@ -735,7 +735,7 @@ func validateCustonNSGNetworking(ctx context.Context, ociCluster infrastructurev Expect(err).NotTo(HaveOccurred()) switch nsg.Role { case infrastructurev1beta1.ControlPlaneEndpointRole: - verifyNsg(ctx, resp, ociClusterOriginal, infrastructurev1beta1.ControlPlaneEndpointRole) + verifyNsg(ctx, resp, ociClusterOriginal, &ociCluster, infrastructurev1beta1.ControlPlaneEndpointRole) lbId := ociCluster.Spec.NetworkSpec.APIServerLB.LoadBalancerId lb, err := lbClient.GetNetworkLoadBalancer(ctx, networkloadbalancer.GetNetworkLoadBalancerRequest{ NetworkLoadBalancerId: lbId, @@ -743,15 +743,15 @@ func validateCustonNSGNetworking(ctx context.Context, ociCluster infrastructurev Expect(err).NotTo(HaveOccurred()) Expect(lb.NetworkSecurityGroupIds[0]).To(Equal(*nsgId)) case infrastructurev1beta1.ControlPlaneRole: - verifyNsg(ctx, resp, ociClusterOriginal, infrastructurev1beta1.ControlPlaneRole) + verifyNsg(ctx, resp, ociClusterOriginal, &ociCluster, infrastructurev1beta1.ControlPlaneRole) Log("Validating control plane machine vnic NSG") validateVnicNSG(ctx, clusterName, nameSpace, nsgId, "") case infrastructurev1beta1.WorkerRole: - verifyNsg(ctx, resp, ociClusterOriginal, infrastructurev1beta1.WorkerRole) + verifyNsg(ctx, resp, ociClusterOriginal, &ociCluster, infrastructurev1beta1.WorkerRole) Log("Validating node machine vnic NSG") validateVnicNSG(ctx, clusterName, nameSpace, nsgId, machineDeployment) case infrastructurev1beta1.ServiceLoadBalancerRole: - verifyNsg(ctx, resp, ociClusterOriginal, infrastructurev1beta1.ServiceLoadBalancerRole) + verifyNsg(ctx, resp, ociClusterOriginal, &ociCluster, infrastructurev1beta1.ServiceLoadBalancerRole) default: return errors.New("invalid nsg role") } @@ -884,12 +884,12 @@ func verifySeclistSubnet(ctx context.Context, resp core.GetSubnetResponse, ociCl Expect(matches).To(Equal(1)) } -func verifyNsg(ctx context.Context, resp core.GetNetworkSecurityGroupResponse, ociClusterOriginal *infrastructurev1beta1.OCICluster, role infrastructurev1beta1.Role) { +func verifyNsg(ctx context.Context, resp core.GetNetworkSecurityGroupResponse, ociClusterOriginal *infrastructurev1beta1.OCICluster, ociClusterActual *infrastructurev1beta1.OCICluster, role infrastructurev1beta1.Role) { listResponse, err := vcnClient.ListNetworkSecurityGroupSecurityRules(ctx, core.ListNetworkSecurityGroupSecurityRulesRequest{ NetworkSecurityGroupId: resp.Id, }) Expect(err).NotTo(HaveOccurred()) - ingressRules, egressRules := generateSpecFromSecurityRules(ociClusterOriginal, listResponse.Items) + ingressRules, egressRules := generateSpecFromSecurityRules(ociClusterActual, listResponse.Items) matches := 0 for _, n := range ociClusterOriginal.Spec.NetworkSpec.Vcn.NetworkSecurityGroups { if n.Role == role {