From c03258a8678f857c023b21afe4d421655f6d374f 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 | 117 +++++++------- ...ster_securitygroup_controller_unit_test.go | 4 +- hack/verify-shellcheck.sh | 4 +- testenv/osccluster_controller_test.go | 145 +++++++++++++----- 5 files changed, 176 insertions(+), 99 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..abfc5d0d3 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,25 @@ 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") } + // Fetch or initialize security group rule references securityGroupRuleRef := clusterScope.GetSecurityGroupRuleRef() if len(securityGroupRuleRef.ResourceMap) == 0 { securityGroupRuleRef.ResourceMap = make(map[string]string) @@ -291,14 +304,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 +320,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..9bb1e47f7 100644 --- a/testenv/osccluster_controller_test.go +++ b/testenv/osccluster_controller_test.go @@ -77,16 +77,19 @@ 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") + waitOscInfraClusterToBeReady(ctx, oscInfraClusterKey) - waitOscClusterToProvision(ctx, capoClusterKey) + clusterScope, err := getClusterScope(ctx, capoClusterKey, oscInfraClusterKey) Expect(err).ShouldNot(HaveOccurred()) + + By("Ensuring network components are provisioned") checkOscNetToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) checkOscSubnetToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) checkOscInternetServiceToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) @@ -94,28 +97,41 @@ func createCheckDeleteOscCluster(ctx context.Context, infraClusterSpec infrastru checkOscPublicIpToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) checkOscRouteTableToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) checkOscRouteToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) + + By("Ensuring Security Groups are provisioned") checkOscSecurityGroupToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) checkOscSecurityGroupRuleToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) + + By("Validating SecurityGroupsRef is not empty") + securityGroupsRef := clusterScope.GetSecurityGroupsRef() + Expect(len(securityGroupsRef.ResourceMap)).To(BeNumerically(">", 0), "SecurityGroupsRef.ResourceMap should not be empty") + + By("Waiting for cluster to provision") + waitOscClusterToProvision(ctx, capoClusterKey) + checkOscLoadBalancerToBeProvisioned(ctx, oscInfraClusterKey, clusterScope) + By("Delete cluster") deleteObj(ctx, oscInfraCluster, oscInfraClusterKey, "oscInfraCluster", "default") 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 +139,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") } @@ -583,62 +630,90 @@ 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 { + // Fetch the NetId and Security Group Service netSpec := clusterScope.GetNet() netId := netSpec.ResourceId securitysvc := security.NewService(ctx, clusterScope) + + // Get Security Groups Spec and IDs securityGroupsSpec := clusterScope.GetSecurityGroups() securityGroupIds, err := securitysvc.GetSecurityGroupIdsFromNetIds(netId) - fmt.Fprintf(GinkgoWriter, "Check SecurityGroupIds received %v \n", securityGroupIds) if err != nil { - return err + return fmt.Errorf("failed to retrieve security group IDs: %w", err) } + + // Populate the ResourceMap if it's empty + securityGroupsRef := clusterScope.GetSecurityGroupsRef() + if len(securityGroupsRef.ResourceMap) == 0 { + for _, securityGroupSpec := range securityGroupsSpec { + if controllers.Contains(securityGroupIds, securityGroupSpec.ResourceId) { + securityGroupsRef.ResourceMap[securityGroupSpec.Name] = securityGroupSpec.ResourceId + fmt.Fprintf(GinkgoWriter, "Populated SecurityGroup ResourceMap for %s\n", securityGroupSpec.Name) + } else { + return fmt.Errorf("security group %s not found in retrieved IDs", securityGroupSpec.Name) + } + } + } + + // Validate that all security groups exist for _, securityGroupSpec := range securityGroupsSpec { securityGroupId := securityGroupSpec.ResourceId - fmt.Fprintf(GinkgoWriter, "Check SecurityGroupId %s\n", securityGroupId) if !controllers.Contains(securityGroupIds, securityGroupId) { - return fmt.Errorf("SecurityGroupId %s does not exist", securityGroupId) + return fmt.Errorf("security group %s with ID %s does not exist", securityGroupSpec.Name, securityGroupId) } } - fmt.Fprintf(GinkgoWriter, "Found OscSecurityGroup \n") - return nil + fmt.Fprintf(GinkgoWriter, "Found OscSecurityGroup and populated ResourceMap\n") + clusterScope.SetSecurityGroupsRef(*securityGroupsRef) + return nil }, 5*time.Minute, 1*time.Second).Should(BeNil()) } // checkOscSecurityGroupRuleToBeProvisioned will validate that OscSecurityGroupRule is provisionned func checkOscSecurityGroupRuleToBeProvisioned(ctx context.Context, oscInfraClusterKey client.ObjectKey, clusterScope *scope.ClusterScope) { - By("Check OscSecurityGroupRule is provisioned") + By("Check OscSecurityGroupRules are provisioned") Eventually(func() error { securitysvc := security.NewService(ctx, clusterScope) + + // Get Security Groups Reference + securityGroupsRef := clusterScope.GetSecurityGroupsRef() + if len(securityGroupsRef.ResourceMap) == 0 { + return fmt.Errorf("securityGroupsRef.ResourceMap is empty, security groups should be reconciled first") + } + + // Validate SecurityGroup Rules securityGroupsSpec := clusterScope.GetSecurityGroups() for _, securityGroupSpec := range securityGroupsSpec { - securityGroupId := securityGroupSpec.ResourceId - fmt.Fprintf(GinkgoWriter, "Check SecurityGroupId %s\n", securityGroupId) + securityGroupId, exists := securityGroupsRef.ResourceMap[securityGroupSpec.Name] + if !exists { + return fmt.Errorf("security group %s not found in ResourceMap", 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) + ruleName := fmt.Sprintf("%s-%s", securityGroupRuleSpec.Name, clusterScope.GetUID()) + fmt.Fprintf(GinkgoWriter, "Checking rule %s for security group %s\n", ruleName, securityGroupId) + + // Validate Rule exists Flow := securityGroupRuleSpec.Flow IpProtocol := securityGroupRuleSpec.IpProtocol IpRange := securityGroupRuleSpec.IpRange FromPortRange := securityGroupRuleSpec.FromPortRange ToPortRange := securityGroupRuleSpec.ToPortRange - securityGroupFromSecurityGroupRule, err := securitysvc.GetSecurityGroupFromSecurityGroupRule(securityGroupId, Flow, IpProtocol, IpRange, "", FromPortRange, ToPortRange) + + _, 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) + return fmt.Errorf("failed to validate rule %s for security group %s: %w", ruleName, securityGroupId, err) } - } } - fmt.Fprintf(GinkgoWriter, "Found OscSecurityGroupRule \n") + fmt.Fprintf(GinkgoWriter, "All security group rules are provisioned\n") return nil }, 5*time.Minute, 1*time.Second).Should(BeNil()) }