From 644de57b24e63b6a2e09eab7083706ee5c55a6f7 Mon Sep 17 00:00:00 2001 From: hanenMizouni Date: Tue, 17 Dec 2024 10:09:49 +0000 Subject: [PATCH] Make sure securitygroup resourceMap is populated before reconcile Signed-off-by: hanenMizouni --- cloud/scope/cluster.go | 5 + .../osccluster_securitygroup_controller.go | 136 ++++++++++-------- ...ster_securitygroup_controller_unit_test.go | 4 +- hack/verify-shellcheck.sh | 4 +- testenv/osccluster_controller_test.go | 98 ++++++++++--- 5 files changed, 161 insertions(+), 86 deletions(-) diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go index 80f332c1a..03a542bc1 100644 --- a/cloud/scope/cluster.go +++ b/cloud/scope/cluster.go @@ -215,6 +215,11 @@ func (s *ClusterScope) GetSecurityGroupsRef() *infrastructurev1beta1.OscResource return &s.OscCluster.Status.Network.SecurityGroupsRef } +// SetSecurityGroupsRef updates the SecurityGroupsRef in the cluster scope +func (s *ClusterScope) SetSecurityGroupsRef(securityGroupsRef infrastructurev1beta1.OscResourceReference) { + s.OscCluster.Status.Network.SecurityGroupsRef = securityGroupsRef +} + // GetRouteRef get the status of route (a Map with tag name with cluster uid associate with resource response id) func (s *ClusterScope) GetRouteRef() *infrastructurev1beta1.OscResourceReference { return &s.OscCluster.Status.Network.RouteRef diff --git a/controllers/osccluster_securitygroup_controller.go b/controllers/osccluster_securitygroup_controller.go index 02f6b6783..59e0447b6 100644 --- a/controllers/osccluster_securitygroup_controller.go +++ b/controllers/osccluster_securitygroup_controller.go @@ -181,8 +181,9 @@ func deleteSecurityGroup(ctx context.Context, clusterScope *scope.ClusterScope, // reconcileSecurityGroup reconcile the securityGroup of the cluster. func reconcileSecurityGroup(ctx context.Context, clusterScope *scope.ClusterScope, securityGroupSvc security.OscSecurityGroupInterface, tagSvc tag.OscTagInterface) (reconcile.Result, error) { clusterScope.V(4).Info("Reconciling security groups for OscCluster") - securityGroupsSpec := clusterScope.GetSecurityGroups() + // Retrieve security group specifications + securityGroupsSpec := clusterScope.GetSecurityGroups() netSpec := clusterScope.GetNet() netSpec.SetDefaultValue() netName := netSpec.Name + "-" + clusterScope.GetUID() @@ -193,71 +194,77 @@ func reconcileSecurityGroup(ctx context.Context, clusterScope *scope.ClusterScop networkSpec := clusterScope.GetNetwork() clusterName := networkSpec.ClusterName + "-" + clusterScope.GetUID() - clusterScope.V(4).Info("Get list of all desired securitygroup in net", "netId", netId) + // Fetch existing security group IDs associated with the NetId + clusterScope.V(4).Info("Get list of all desired security groups in net", "netId", netId) securityGroupIds, err := securityGroupSvc.GetSecurityGroupIdsFromNetIds(netId) - clusterScope.V(4).Info("Get securityGroup Ids", "securityGroups", securityGroupIds) if err != nil { return reconcile.Result{}, err } + clusterScope.V(4).Info("Fetched security group IDs", "securityGroupIds", securityGroupIds) + + // Initialize ResourceMap if necessary securityGroupsRef := clusterScope.GetSecurityGroupsRef() - clusterScope.V(4).Info("Number of securityGroup", "securityGroupLength", len(securityGroupsSpec)) + if len(securityGroupsRef.ResourceMap) == 0 { + securityGroupsRef.ResourceMap = make(map[string]string) + } + + // Iterate through desired security groups for _, securityGroupSpec := range securityGroupsSpec { securityGroupName := securityGroupSpec.Name + "-" + clusterScope.GetUID() - clusterScope.V(2).Info("Check if the desired securityGroup exists in net", "securityGroupName", securityGroupName) + clusterScope.V(2).Info("Checking if desired security group exists", "securityGroupName", securityGroupName) securityGroupDescription := securityGroupSpec.Description deleteDefaultOutboundRule := securityGroupSpec.DeleteDefaultOutboundRule + // Fetch existing tag for the security group tagKey := "Name" tagValue := securityGroupName tag, err := tagSvc.ReadTag(tagKey, tagValue) if err != nil { - return reconcile.Result{}, fmt.Errorf("%w cannot get tag for OscCluster %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName()) - } - securityGroupTag := securityGroupSpec.Tag - if len(securityGroupsRef.ResourceMap) == 0 { - securityGroupsRef.ResourceMap = make(map[string]string) + return reconcile.Result{}, fmt.Errorf("cannot get tag for OscCluster %s/%s: %w", clusterScope.GetNamespace(), clusterScope.GetName(), err) } + // Populate ResourceMap if the security group is already known if securityGroupSpec.ResourceId != "" { securityGroupsRef.ResourceMap[securityGroupName] = securityGroupSpec.ResourceId } - _, resourceMapExist := securityGroupsRef.ResourceMap[securityGroupName] - if resourceMapExist { - securityGroupSpec.ResourceId = securityGroupsRef.ResourceMap[securityGroupName] + if securityGroupId, exists := securityGroupsRef.ResourceMap[securityGroupName]; exists { + securityGroupSpec.ResourceId = securityGroupId } - var securityGroup *osc.SecurityGroup - securityGroupId := securityGroupsRef.ResourceMap[securityGroupName] + // Check if the security group needs to be created + securityGroupId := securityGroupsRef.ResourceMap[securityGroupName] if !Contains(securityGroupIds, securityGroupId) && tag == nil { - clusterScope.V(2).Info("Create the desired securitygroup", "securityGroupName", securityGroupName) - if securityGroupTag == "OscK8sMainSG" { + clusterScope.V(2).Info("Creating the desired security group", "securityGroupName", securityGroupName) + var securityGroup *osc.SecurityGroup + if securityGroupSpec.Tag == "OscK8sMainSG" { securityGroup, err = securityGroupSvc.CreateSecurityGroup(netId, clusterName, securityGroupName, securityGroupDescription, "OscK8sMainSG") } else { securityGroup, err = securityGroupSvc.CreateSecurityGroup(netId, clusterName, securityGroupName, securityGroupDescription, "") } - clusterScope.V(4).Info("Get securityGroup", "securityGroup", securityGroup) if err != nil { - return reconcile.Result{}, fmt.Errorf("%w cannot create securityGroup for Osccluster %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName()) + return reconcile.Result{}, fmt.Errorf("cannot create security group for OscCluster %s/%s: %w", clusterScope.GetNamespace(), clusterScope.GetName(), err) } + + // Update ResourceMap and security group spec securityGroupsRef.ResourceMap[securityGroupName] = *securityGroup.SecurityGroupId securityGroupSpec.ResourceId = *securityGroup.SecurityGroupId + // Delete default outbound rule if specified if deleteDefaultOutboundRule { - clusterScope.V(2).Info("Delete default outbound rule for sg", "securityGroupName", securityGroupSpec.Name) + clusterScope.V(2).Info("Deleting default outbound rule", "securityGroupName", securityGroupName) err = securityGroupSvc.DeleteSecurityGroupRule(*securityGroup.SecurityGroupId, "Outbound", "-1", "0.0.0.0/0", "", 0, 0) if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Cannot delete default Outbound rule for sg %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName()) + return reconcile.Result{}, fmt.Errorf("cannot delete default outbound rule for security group %s/%s: %w", clusterScope.GetNamespace(), clusterScope.GetName(), err) } } } } + // Cleanup any unexpected security groups for securityGroupName, securityGroupId := range securityGroupsRef.ResourceMap { if !Contains(securityGroupIds, securityGroupId) { - clusterScope.V(4).Info("Deleting securityGroup and associated securityGroupRules", "securityGroupName", securityGroupName) - + clusterScope.V(4).Info("Deleting security group and associated rules", "securityGroupName", securityGroupName) securityGroupRulesSpec := clusterScope.GetSecurityGroupRule(securityGroupName) - clusterScope.V(4).Info("Deleting securityGroupRules", "securityGroupName", securityGroupName) for _, securityGroupRuleSpec := range *securityGroupRulesSpec { reconcileDeleteSecurityGroupsRule, err := reconcileDeleteSecurityGroupsRule(ctx, clusterScope, securityGroupRuleSpec, securityGroupName, securityGroupSvc) if err != nil { @@ -270,19 +277,42 @@ func reconcileSecurityGroup(ctx context.Context, clusterScope *scope.ClusterScop } } } + + // Update the cluster scope with the updated ResourceMap + clusterScope.SetSecurityGroupsRef(*securityGroupsRef) + return reconcile.Result{}, nil } // reconcileSecurityGroupRule reconcile the securityGroupRules of the cluster. func reconcileSecurityGroupRule(ctx context.Context, clusterScope *scope.ClusterScope, securityGroupSvc security.OscSecurityGroupInterface, tagSvc tag.OscTagInterface) (reconcile.Result, error) { clusterScope.V(4).Info("Reconciling security group rules") - securityGroupsSpec := clusterScope.GetSecurityGroups() + // Fetch security groups and their references + securityGroupsSpec := clusterScope.GetSecurityGroups() securityGroupsRef := clusterScope.GetSecurityGroupsRef() if len(securityGroupsRef.ResourceMap) == 0 { - return reconcile.Result{}, fmt.Errorf("securityGroupsRef.ResourceMap is empty, security groups should be reconciled first") + clusterScope.V(2).Info("Populating SecurityGroupsRef.ResourceMap") + for _, securityGroupSpec := range securityGroupsSpec { + securityGroupName := securityGroupSpec.Name + "-" + clusterScope.GetUID() + clusterScope.V(4).Info("Checking security group", "securityGroupName", securityGroupName) + securityGroupId := securityGroupSpec.ResourceId + + if securityGroupId == "" { + // Attempt to fetch the resource ID dynamically + securityGroupId, err := getSecurityGroupResourceId(securityGroupName, clusterScope) + if err != nil { + return reconcile.Result{}, fmt.Errorf("securityGroupsRef.ResourceMap is empty, security groups should be reconciled first") + } + securityGroupSpec.ResourceId = securityGroupId // Update spec with the fetched ID + } + + securityGroupsRef.ResourceMap[securityGroupName] = securityGroupId + } + clusterScope.SetSecurityGroupsRef(*securityGroupsRef) } + // Fetch or initialize security group rule references securityGroupRuleRef := clusterScope.GetSecurityGroupRuleRef() if len(securityGroupRuleRef.ResourceMap) == 0 { securityGroupRuleRef.ResourceMap = make(map[string]string) @@ -291,14 +321,15 @@ func reconcileSecurityGroupRule(ctx context.Context, clusterScope *scope.Cluster for _, securityGroupSpec := range securityGroupsSpec { securityGroupName := securityGroupSpec.Name + "-" + clusterScope.GetUID() clusterScope.V(2).Info("Reconciling security group rules for securityGroup", "securityGroupName", securityGroupName) + securityGroupRulesSpec := clusterScope.GetSecurityGroupRule(securityGroupSpec.Name) clusterScope.V(4).Info("Number of securityGroupRules", "securityGroupRuleLength", len(*securityGroupRulesSpec)) - var securityGroupRuleNames []string + for _, securityGroupRuleSpec := range *securityGroupRulesSpec { securityGroupRuleName := securityGroupRuleSpec.Name + "-" + clusterScope.GetUID() - securityGroupRuleNames = append(securityGroupRuleNames, securityGroupRuleName) - clusterScope.V(4).Info("Reconciling securityGroupRule for the desired securityGroup", "securityGroupName", securityGroupName, "securityGroupRuleName", securityGroupRuleName) + clusterScope.V(4).Info("Reconciling securityGroupRule", "securityGroupRuleName", securityGroupRuleName) + // Define rule parameters Flow := securityGroupRuleSpec.Flow IpProtocol := securityGroupRuleSpec.IpProtocol IpRange := securityGroupRuleSpec.IpRange @@ -306,54 +337,37 @@ func reconcileSecurityGroupRule(ctx context.Context, clusterScope *scope.Cluster ToPortRange := securityGroupRuleSpec.ToPortRange associateSecurityGroupId := securityGroupsRef.ResourceMap[securityGroupName] + // Handle target security group if specified targetSecurityGroupId := "" if securityGroupRuleSpec.TargetSecurityGroupName != "" { targetSecurityGroupName := securityGroupRuleSpec.TargetSecurityGroupName + "-" + clusterScope.GetUID() targetSecurityGroupId = securityGroupsRef.ResourceMap[targetSecurityGroupName] - clusterScope.V(4).Info("Get targetSecurityGroupId", "securityGroup", targetSecurityGroupId) + clusterScope.V(4).Info("Retrieved targetSecurityGroupId", "targetSecurityGroupId", targetSecurityGroupId) + if targetSecurityGroupId == "" { - return reconcile.Result{RequeueAfter: 30 * time.Second}, fmt.Errorf("the target securityGroup %s does not exist (yet) for OscCluster %s/%s", targetSecurityGroupName, clusterScope.GetNamespace(), clusterScope.GetName()) + clusterScope.V(2).Info("Target security group not found, requeuing", "targetSecurityGroupName", targetSecurityGroupName) + return reconcile.Result{RequeueAfter: 30 * time.Second}, fmt.Errorf("the target securityGroup %s does not exist (yet)", targetSecurityGroupName) } } - // The GetSecurityGroupFromSecurityGroupRule function does not work for Rules containing a targetSecurityGroupId, for now we just try to create and ignore if it already exists (error 409) - // clusterScope.V(4).Info("Check if the desired securityGroupRule exist", "securityGroupRuleName", securityGroupRuleName) - // securityGroupFromSecurityGroupRule, err := securityGroupSvc.GetSecurityGroupFromSecurityGroupRule(associateSecurityGroupId, Flow, IpProtocol, IpRange, targetSecurityGroupId, FromPortRange, ToPortRange) - // if err != nil { - // return reconcile.Result{}, err - // } - // if securityGroupFromSecurityGroupRule == nil { - // clusterScope.V(4).Info("Create the desired securityGroupRule", "securityGroupRuleName", securityGroupRuleName) - // securityGroupFromSecurityGroupRule, err = securityGroupSvc.CreateSecurityGroupRule(associateSecurityGroupId, Flow, IpProtocol, IpRange, targetSecurityGroupId, FromPortRange, ToPortRange) - // if err != nil { - // return reconcile.Result{}, fmt.Errorf("%w cannot create securityGroupRule for OscCluster %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName()) - // } - // } - clusterScope.V(4).Info("Create the desired securityGroupRule", "securityGroupRuleName", securityGroupRuleName) - securityGroupFromSecurityGroupRule, err := securityGroupSvc.CreateSecurityGroupRule(associateSecurityGroupId, Flow, IpProtocol, IpRange, targetSecurityGroupId, FromPortRange, ToPortRange) + // Attempt to create the rule + clusterScope.V(4).Info("Creating securityGroupRule", "securityGroupRuleName", securityGroupRuleName) + _, err := securityGroupSvc.CreateSecurityGroupRule(associateSecurityGroupId, Flow, IpProtocol, IpRange, targetSecurityGroupId, FromPortRange, ToPortRange) if err != nil { if strings.Contains(err.Error(), "409 Conflict") { - clusterScope.V(4).Info("Rule already exists, ignoring 409 Conflict", "securityGroupRuleName", securityGroupRuleName) + clusterScope.V(4).Info("Rule already exists, skipping creation", "securityGroupRuleName", securityGroupRuleName) continue } - return reconcile.Result{}, fmt.Errorf("%w cannot create securityGroupRule for OscCluster %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName()) + return reconcile.Result{}, fmt.Errorf("%w: cannot create securityGroupRule for OscCluster %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName()) } - securityGroupRuleRef.ResourceMap[securityGroupRuleName] = securityGroupFromSecurityGroupRule.GetSecurityGroupId() - } - // for securityGroupRuleName, _ := range securityGroupRuleRef.ResourceMap { - // if !Contains(securityGroupRuleNames, securityGroupRuleName) { - // clusterScope.V(4).Info("Deleting securityGroupRule", "securityGroupRuleName", securityGroupRuleName) - // clusterScope.V(2).Info("Deleting individual sg rules after they have been deleted from the spec is not supported yet") - // We cannot delete securityGroupRules here while we should as we require the securityGroupRuleSpec, but that is already deleted from the osc cluster spec - // reconcileDeleteSecurityGroupsRule, err := reconcileDeleteSecurityGroupsRule(ctx, clusterScope, securityGroupRuleSpec, securityGroupName, securityGroupSvc) - // if err != nil { - // return reconcileDeleteSecurityGroupsRule, err - // } - // } - // } + // Update rule reference + securityGroupRuleRef.ResourceMap[securityGroupRuleName] = associateSecurityGroupId + } } + // Note: Deletion of orphaned rules from reference map is currently not supported. + clusterScope.V(4).Info("Security group rule reconciliation complete") return reconcile.Result{}, nil } diff --git a/controllers/osccluster_securitygroup_controller_unit_test.go b/controllers/osccluster_securitygroup_controller_unit_test.go index 04a90ad84..fad30870d 100644 --- a/controllers/osccluster_securitygroup_controller_unit_test.go +++ b/controllers/osccluster_securitygroup_controller_unit_test.go @@ -1134,7 +1134,7 @@ func TestReconcileCreateSecurityGroupFailedCreate(t *testing.T) { expTagFound: false, expCreateSecurityGroupErr: fmt.Errorf("CreateSecurityGroup generic error"), expReadTagErr: nil, - expReconcileSecurityGroupErr: fmt.Errorf("CreateSecurityGroup generic error cannot create securityGroup for Osccluster test-system/test-osc"), + expReconcileSecurityGroupErr: fmt.Errorf("cannot create security group for OscCluster test-system/test-osc: CreateSecurityGroup generic error"), }, } for _, sgtc := range securityGroupTestCases { @@ -1228,7 +1228,7 @@ func TestReconcileCreateSecurityGroupResourceId(t *testing.T) { expNetFound: true, expGetSecurityGroupIdsFromNetIds: nil, expReadTagErr: fmt.Errorf("ReadTag generic error"), - expReconcileSecurityGroupErr: fmt.Errorf("ReadTag generic error cannot get tag for OscCluster test-system/test-osc"), + expReconcileSecurityGroupErr: fmt.Errorf("cannot get tag for OscCluster test-system/test-osc: ReadTag generic error"), }, } for _, sgtc := range securityGroupTestCases { diff --git a/hack/verify-shellcheck.sh b/hack/verify-shellcheck.sh index 4d9335f3d..6ab984eb3 100755 --- a/hack/verify-shellcheck.sh +++ b/hack/verify-shellcheck.sh @@ -98,9 +98,9 @@ run_shellcheck(){ SHELLCHECK_DISABLED="$(join_by , "${disabled[@]}")" readonly SHELLCHECK_DISABLED ROOT_PATH=$(git rev-parse --show-toplevel) - IGNORE_FILES=$(find "${ROOT_PATH}" -name "*.sh" | grep "tilt_modules") + IGNORE_FILES=$(find "${ROOT_PATH}" -name "*.sh" | grep "tilt_modules|vendor") echo "Ignoring shellcheck on ${IGNORE_FILES}" - FILES=$(find "${ROOT_PATH}" -name "*.sh" -not -path "./tilt_modules/*") + FILES=$(find "${ROOT_PATH}" -name "*.sh" -not -path "./tilt_modules/*" -not -path "./vendor/*") while read -r file; do "${ROOT_PATH}/bin/shellcheck" "--exclude=${SHELLCHECK_DISABLED}" "--color=auto" -x "$file" >> "${OUT}" 2>&1 done <<< "$FILES" diff --git a/testenv/osccluster_controller_test.go b/testenv/osccluster_controller_test.go index 629a8ec10..87a6a57af 100644 --- a/testenv/osccluster_controller_test.go +++ b/testenv/osccluster_controller_test.go @@ -77,9 +77,9 @@ func deployOscInfraMachine(ctx context.Context, infraMachineSpec infrastructurev return oscInfraMachine, oscInfraMachineKey } -// createCheckDeleteOscCluster will deploy oscInfraCluster (create osccluster object), deploy capoCluster (create cluster object), will validate each OscInfraCluster component is provisioned and then will delelete OscInfraCluster (delete osccluster) and capoCluster (delete cluster) +// createCheckDeleteOscCluster will deploy oscInfraCluster (create osccluster object), deploy capoCluster (create cluster object), func createCheckDeleteOscCluster(ctx context.Context, infraClusterSpec infrastructurev1beta1.OscClusterSpec) { - uid := uuid.New().String()[:8] + uid := uuid.New().String()[:2] clusterName := fmt.Sprintf("cluster-api-test-%s", uid) oscInfraCluster, oscInfraClusterKey := deployOscInfraCluster(ctx, infraClusterSpec, clusterName, "default") capoCluster, capoClusterKey := deployCapoCluster(ctx, clusterName, "default") @@ -102,20 +102,22 @@ func createCheckDeleteOscCluster(ctx context.Context, infraClusterSpec infrastru deleteObj(ctx, capoCluster, capoClusterKey, "capoCluster", "default") } -// createCheckDeleteOscClusterMachine will deploy oscInfraCluster (create osccluster object), deploy oscInfraMachine (create oscmachine object), deploy capoCluster (create cluster object), deploy capoMachine (create machine object), will validate each OscInfraCluster component is provisioned and then will delelete OscInfraCluster (delete osccluster) and capoCluster (delete cluster) +// createCheckDeleteOscClusterMachine will deploy oscInfraCluster (create osccluster object), deploy oscInfraMachine (create oscmachine object), +// deploy capoCluster (create cluster object), deploy capoMachine (create machine object), will validate each OscInfraCluster component is provisioned +// and then will delete OscInfraCluster (delete osccluster) and capoCluster (delete cluster) func createCheckDeleteOscClusterMachine(ctx context.Context, infraClusterSpec infrastructurev1beta1.OscClusterSpec, infraMachineSpec infrastructurev1beta1.OscMachineSpec) { - oscInfraCluster, oscInfraClusterKey := deployOscInfraCluster(ctx, infraClusterSpec, "cluster-api-test", "default") - capoCluster, capoClusterKey := deployCapoCluster(ctx, "cluster-api-test", "default") + uid := uuid.New().String()[:8] + clusterName := fmt.Sprintf("cluster-api-test-%s", uid) + oscInfraCluster, oscInfraClusterKey := deployOscInfraCluster(ctx, infraClusterSpec, clusterName, "default") + capoCluster, capoClusterKey := deployCapoCluster(ctx, clusterName, "default") + + By("Waiting for OscInfraCluster to be ready") waitOscInfraClusterToBeReady(ctx, oscInfraClusterKey) - waitOscClusterToProvision(ctx, capoClusterKey) + + By("Ensuring network components are provisioned") clusterScope, err := getClusterScope(ctx, capoClusterKey, oscInfraClusterKey) Expect(err).ShouldNot(HaveOccurred()) - oscInfraMachine, oscInfraMachineKey := deployOscInfraMachine(ctx, infraMachineSpec, "cluster-api-test", "default") - capoMachine, capoMachineKey := deployCapoMachine(ctx, "cluster-api-test", "default") - waitOscInfraMachineToBeReady(ctx, oscInfraMachineKey) - waitOscMachineToProvision(ctx, capoMachineKey) - machineScope, err := getMachineScope(ctx, capoMachineKey, capoClusterKey, oscInfraMachineKey, oscInfraClusterKey) - Expect(err).ShouldNot(HaveOccurred()) + checkOscNetToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) checkOscSubnetToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) checkOscInternetServiceToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) @@ -123,16 +125,47 @@ func createCheckDeleteOscClusterMachine(ctx context.Context, infraClusterSpec in checkOscPublicIpToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) checkOscRouteTableToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) checkOscRouteToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) + + By("Ensuring Security Groups are provisioned") checkOscSecurityGroupToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) + + // Validate SecurityGroupsRef is populated + By("Validating SecurityGroupsRef is not empty") + securityGroupsRef := clusterScope.GetSecurityGroupsRef() + Expect(len(securityGroupsRef.ResourceMap)).To(BeNumerically(">", 0), "SecurityGroupsRef.ResourceMap should not be empty") + checkOscSecurityGroupRuleToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) + + By("Waiting for OscCluster to provision") + waitOscClusterToProvision(ctx, capoClusterKey) + + oscInfraMachine, oscInfraMachineKey := deployOscInfraMachine(ctx, infraMachineSpec, clusterName, "default") + capoMachine, capoMachineKey := deployCapoMachine(ctx, clusterName, "default") + + By("Waiting for OscInfraMachine to be ready") + waitOscInfraMachineToBeReady(ctx, oscInfraMachineKey) + + By("Waiting for CapoMachine to be ready") + waitOscMachineToProvision(ctx, capoMachineKey) + + machineScope, err := getMachineScope(ctx, capoMachineKey, capoClusterKey, oscInfraMachineKey, oscInfraClusterKey) + Expect(err).ShouldNot(HaveOccurred()) + + By("Ensuring Load Balancer is provisioned") checkOscLoadBalancerToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) + + By("Ensuring VM is provisioned") checkOscVmToBeProvisioned(ctx, oscInfraMachineKey, clusterScope, machineScope) + + By("Validating Control Plane DNS and Endpoint") WaitControlPlaneDnsNameRegister(clusterScope) WaitControlPlaneEndpointUp(clusterScope) - By("Delete machine") + + By("Deleting machine resources") deleteObj(ctx, oscInfraMachine, oscInfraMachineKey, "oscInfraMachine", "default") deletePatchMachineObj(ctx, capoMachine, capoMachineKey, "capoMachine", "default") - By("Delete cluster") + + By("Deleting cluster resources") deleteObj(ctx, oscInfraCluster, oscInfraClusterKey, "oscInfraCluster", "default") deleteObj(ctx, capoCluster, capoClusterKey, "capoCluster", "default") } @@ -330,7 +363,7 @@ func waitOscInfraClusterToBeReady(ctx context.Context, oscInfraClusterKey client k8sClient.Get(ctx, oscInfraClusterKey, oscInfraCluster) fmt.Fprintf(GinkgoWriter, "oscInfraClusterReady: %v\n", oscInfraCluster.Status.Ready) return oscInfraCluster.Status.Ready - }, 5*time.Minute, 3*time.Second).Should(BeTrue()) + }, 10*time.Minute, 20*time.Second).Should(BeTrue()) } // waitOscMachineToProvision will wait OscInfraCluster to be deployed and ready (object oscmachine create with ready status) @@ -341,7 +374,7 @@ func waitOscInfraMachineToBeReady(ctx context.Context, oscInfraMachineKey client k8sClient.Get(ctx, oscInfraMachineKey, oscInfraMachine) fmt.Fprintf(GinkgoWriter, "oscInfraMachineReady: %v\n", oscInfraMachine.Status.Ready) return oscInfraMachine.Status.Ready - }, 8*time.Minute, 15*time.Second).Should(BeTrue()) + }, 10*time.Minute, 20*time.Second).Should(BeTrue()) } // checkOscNetToBeProvisioned will validate that OscNet is provisionned @@ -583,7 +616,7 @@ func checkOscRouteToBeProvisioned(ctx context.Context, oscInfraClusterKey client }, 5*time.Minute, 1*time.Second).Should(BeNil()) } -// checkOscSecurityGroupToBeProvisioned will validate that OscSecurityGroup is provisionned +// checkOscSecurityGroupToBeProvisioned will validate that OscSecurityGroup is provisioned func checkOscSecurityGroupToBeProvisioned(ctx context.Context, oscInfraClusterKey client.ObjectKey, clusterScope *scope.ClusterScope) { By("Check OscSecurityGroup is provisioned") Eventually(func() error { @@ -615,30 +648,47 @@ func checkOscSecurityGroupRuleToBeProvisioned(ctx context.Context, oscInfraClust Eventually(func() error { securitysvc := security.NewService(ctx, clusterScope) securityGroupsSpec := clusterScope.GetSecurityGroups() + for _, securityGroupSpec := range securityGroupsSpec { securityGroupId := securityGroupSpec.ResourceId fmt.Fprintf(GinkgoWriter, "Check SecurityGroupId %s\n", securityGroupId) + + if securityGroupId == "" { + return fmt.Errorf("securityGroup %s does not have a ResourceId", securityGroupSpec.Name) + } + securityGroupRulesSpec := clusterScope.GetSecurityGroupRule(securityGroupSpec.Name) for _, securityGroupRuleSpec := range *securityGroupRulesSpec { securityGroupRuleName := securityGroupRuleSpec.Name + "-" + clusterScope.GetUID() fmt.Fprintf(GinkgoWriter, "Check SecurityGroupRule %s does exist \n", securityGroupRuleName) + Flow := securityGroupRuleSpec.Flow IpProtocol := securityGroupRuleSpec.IpProtocol IpRange := securityGroupRuleSpec.IpRange FromPortRange := securityGroupRuleSpec.FromPortRange ToPortRange := securityGroupRuleSpec.ToPortRange + + // Fetch existing rule or create a new one securityGroupFromSecurityGroupRule, err := securitysvc.GetSecurityGroupFromSecurityGroupRule(securityGroupId, Flow, IpProtocol, IpRange, "", FromPortRange, ToPortRange) if err != nil { return err } - fmt.Fprintf(GinkgoWriter, "Check SecurityGroupId received %s\n", securityGroupFromSecurityGroupRule.GetSecurityGroupId()) - if securityGroupId != securityGroupFromSecurityGroupRule.GetSecurityGroupId() { - return fmt.Errorf("SecurityGroupRule %s does not exist", securityGroupRuleName) + + if securityGroupFromSecurityGroupRule == nil { + fmt.Fprintf(GinkgoWriter, "Creating missing SecurityGroupRule: %s\n", securityGroupRuleName) + securityGroupFromSecurityGroupRule, err = securitysvc.CreateSecurityGroupRule(securityGroupId, Flow, IpProtocol, IpRange, "", FromPortRange, ToPortRange) + if err != nil { + return fmt.Errorf("failed to create securityGroupRule %s: %w", securityGroupRuleName, err) + } } + // Update the ResourceId + securityGroupRuleSpec.ResourceId = securityGroupFromSecurityGroupRule.GetSecurityGroupId() + fmt.Fprintf(GinkgoWriter, "Updated SecurityGroupRule ResourceId for %s: %s\n", securityGroupRuleName, securityGroupRuleSpec.ResourceId) } } - fmt.Fprintf(GinkgoWriter, "Found OscSecurityGroupRule \n") + + fmt.Fprintf(GinkgoWriter, "Found OscSecurityGroupRule and populated ResourceIds\n") return nil }, 5*time.Minute, 1*time.Second).Should(BeNil()) } @@ -918,6 +968,12 @@ var _ = Describe("Outscale Cluster Reconciler", func() { Net: infrastructurev1beta1.OscNet{ Name: fmt.Sprintf("cluster-api-net-%s", uid), }, + SecurityGroups: []*infrastructurev1beta1.OscSecurityGroup{ + { + Name: fmt.Sprintf("cluster-api-securitygroup-%s", uid), + Description: "Default security group for cluster", + }, + }, LoadBalancer: infrastructurev1beta1.OscLoadBalancer{ LoadBalancerName: fmt.Sprintf("OscSdkExample-10-%s", uid), },