From 75161179951a478089e4a75141ed792cf4a78942 Mon Sep 17 00:00:00 2001 From: yiraeChristineKim Date: Thu, 1 Aug 2024 09:16:03 -0400 Subject: [PATCH] Refactor name setting for complex cases https://issues.redhat.com/browse/ACM-12563 Signed-off-by: yiraeChristineKim --- docs/policygenerator-reference.yaml | 2 + internal/ordering_test.go | 8 +- .../manifest-extradeps-configpolicy.yaml | 2 +- ...ifest-level-name-raw-consolidate-true.yaml | 77 +++++++++++++++++++ internal/utils.go | 47 +++++++---- 5 files changed, 117 insertions(+), 19 deletions(-) diff --git a/docs/policygenerator-reference.yaml b/docs/policygenerator-reference.yaml index e70e1be..97afea0 100644 --- a/docs/policygenerator-reference.yaml +++ b/docs/policygenerator-reference.yaml @@ -236,6 +236,8 @@ policies: - path: "" # Optional. This name is used when ConsolidateManifests is set to false and will serve as the ConfigurationPolicy name. # If multiple manifests are present in the path, an index number will be appended. + # If multiple manifests are present and their names are provided, with `consolidateManifests` set to true, + # the name of the first manifest will be used for all manifest paths. name: "my-config-name" # Optional. (See policyDefaults.complianceType for description.) complianceType: "musthave" diff --git a/internal/ordering_test.go b/internal/ordering_test.go index b84a4e0..d8a6d9d 100644 --- a/internal/ordering_test.go +++ b/internal/ordering_test.go @@ -293,10 +293,16 @@ metadata: policyDefaults: orderPolicies: true namespace: my-policies - consolidateManifests: false + consolidateManifests: true policies: - name: one manifests: + - path: {{printf "%v/%v" .Dir "configmap.yaml"}} + name: tiger + - path: {{printf "%v/%v" .Dir "configmap.yaml"}} + name: rabbit + - path: {{printf "%v/%v" .Dir "object-templates-raw.yaml"}} + name: bird - path: {{printf "%v/%v" .Dir "object-templates-raw.yaml"}} - path: {{printf "%v/%v" .Dir "object-templates-raw.yaml"}} `, diff --git a/internal/testdata/ordering/manifest-extradeps-configpolicy.yaml b/internal/testdata/ordering/manifest-extradeps-configpolicy.yaml index 30c92f9..836c443 100644 --- a/internal/testdata/ordering/manifest-extradeps-configpolicy.yaml +++ b/internal/testdata/ordering/manifest-extradeps-configpolicy.yaml @@ -38,7 +38,7 @@ spec: apiVersion: policy.open-cluster-management.io/v1 kind: ConfigurationPolicy metadata: - name: one2 + name: one spec: object-templates: - complianceType: musthave diff --git a/internal/testdata/ordering/manifest-level-name-raw-consolidate-true.yaml b/internal/testdata/ordering/manifest-level-name-raw-consolidate-true.yaml index 5f8d9be..e4cd3f8 100644 --- a/internal/testdata/ordering/manifest-level-name-raw-consolidate-true.yaml +++ b/internal/testdata/ordering/manifest-level-name-raw-consolidate-true.yaml @@ -12,6 +12,42 @@ metadata: spec: disabled: false policy-templates: + - objectDefinition: + apiVersion: policy.open-cluster-management.io/v1 + kind: ConfigurationPolicy + metadata: + name: bird + spec: + object-templates-raw: |- + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: ConfigMap + metadata: + name: example + namespace: default + data: + extraData: data + remediationAction: inform + severity: low + - objectDefinition: + apiVersion: policy.open-cluster-management.io/v1 + kind: ConfigurationPolicy + metadata: + name: bird2 + spec: + object-templates-raw: |- + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: ConfigMap + metadata: + name: example + namespace: default + data: + extraData: data + remediationAction: inform + severity: low - objectDefinition: apiVersion: policy.open-cluster-management.io/v1 kind: ConfigurationPolicy @@ -84,6 +120,47 @@ spec: extraData: data remediationAction: inform severity: low + - objectDefinition: + apiVersion: policy.open-cluster-management.io/v1 + kind: ConfigurationPolicy + metadata: + name: tiger + spec: + object-templates: + - complianceType: musthave + objectDefinition: + apiVersion: v1 + data: + game.properties: enemies=potato + kind: ConfigMap + metadata: + name: my-configmap + - complianceType: musthave + objectDefinition: + apiVersion: v1 + data: + game.properties: enemies=cabbage + kind: ConfigMap + metadata: + name: config-2 + - complianceType: musthave + objectDefinition: + apiVersion: v1 + data: + game.properties: enemies=potato + kind: ConfigMap + metadata: + name: my-configmap + - complianceType: musthave + objectDefinition: + apiVersion: v1 + data: + game.properties: enemies=cabbage + kind: ConfigMap + metadata: + name: config-2 + remediationAction: inform + severity: low remediationAction: inform --- apiVersion: cluster.open-cluster-management.io/v1beta1 diff --git a/internal/utils.go b/internal/utils.go index 6e3814d..7615523 100644 --- a/internal/utils.go +++ b/internal/utils.go @@ -169,6 +169,8 @@ func getPolicyTemplates(policyConf *types.PolicyConfig) ([]map[string]interface{ objectTemplates := make([]map[string]interface{}, 0, objectTemplatesLength) policyTemplates := make([]map[string]interface{}, 0, policyTemplatesLength) + policyNameCounter := 0 + for i, manifestGroup := range manifestGroups { complianceType := policyConf.Manifests[i].ComplianceType metadataComplianceType := policyConf.Manifests[i].MetadataComplianceType @@ -177,7 +179,9 @@ func getPolicyTemplates(policyConf *types.PolicyConfig) ([]map[string]interface{ ignorePending := policyConf.Manifests[i].IgnorePending extraDeps := policyConf.Manifests[i].ExtraDependencies - for j, manifest := range manifestGroup { + manifestNameCounter := 0 + + for _, manifest := range manifestGroup { err := setGatekeeperEnforcementAction(manifest, policyConf.Manifests[i].GatekeeperEnforcementAction) if err != nil { @@ -204,8 +208,7 @@ func getPolicyTemplates(policyConf *types.PolicyConfig) ([]map[string]interface{ policyConf, manifest["object-templates-raw"], &policyConf.Manifests[i].ConfigurationPolicyOptions, - getConfigurationPolicyName(policyConf, i, len(policyTemplates), - j, !policyConf.ConsolidateManifests), + getConfigurationPolicyName(policyConf, i, &policyNameCounter, &manifestNameCounter), ) } else { policyTemplate = map[string]interface{}{"objectDefinition": manifest} @@ -262,8 +265,8 @@ func getPolicyTemplates(policyConf *types.PolicyConfig) ([]map[string]interface{ policyConf, []map[string]interface{}{objTemplate}, &policyConf.Manifests[i].ConfigurationPolicyOptions, - getConfigurationPolicyName(policyConf, i, len(policyTemplates), - j, !policyConf.ConsolidateManifests), + getConfigurationPolicyName(policyConf, i, + &policyNameCounter, &manifestNameCounter), ) setTemplateOptions(policyTemplate, ignorePending, extraDeps) @@ -282,11 +285,13 @@ func getPolicyTemplates(policyConf *types.PolicyConfig) ([]map[string]interface{ // just build one policyTemplate by using the above non-empty consolidated objectTemplates // ConsolidateManifests = true or there is non-policy-type manifest if policyConf.ConsolidateManifests && len(objectTemplates) > 0 { + // If ConsolidateManifests is true and multiple manifest[].names are provided, the configuration policy + // name will be the first name of manifest[].names policyTemplate := buildPolicyTemplate( policyConf, objectTemplates, &policyConf.ConfigurationPolicyOptions, - getConfigurationPolicyName(policyConf, 0, 0, 0, false), + getConfigurationPolicyName(policyConf, 0, &policyNameCounter, nil), ) setTemplateOptions(policyTemplate, policyConf.IgnorePending, policyConf.ExtraDependencies) policyTemplates = append(policyTemplates, policyTemplate) @@ -322,27 +327,35 @@ func getPolicyTemplates(policyConf *types.PolicyConfig) ([]map[string]interface{ return policyTemplates, nil } -// This function returns configurationPolicyName with Index+1 based on the provided PolicyConfig. -// When index is 0, it won't attach any number. When useManifestName is true, -// it returns the policyConf.Manifests[manifestGroupIndex].Name otherwise, -// It returns policyConf.Name + index -func getConfigurationPolicyName(policyConf *types.PolicyConfig, manifestGroupIndex, policyNum, - manifestIndex int, useManifestName bool, +// getConfigurationPolicyName returns the `ConfigurationPolicy` name based on the provided PolicyConfig. +// When a name is assigned, it increments the associated counter, using it as a suffix if it's greater +// than one to create unique names. If both `manifest[].name` and `policies.name` are provided, +// `manifest[].name` takes priority as the configuration name. +func getConfigurationPolicyName(policyConf *types.PolicyConfig, manifestGroupIndex int, + policyNameCounter *int, manifestNameCounter *int, ) string { // Do not append a number to configName when it is used for the first time. configName := policyConf.Manifests[manifestGroupIndex].Name - if useManifestName && configName != "" { - if manifestIndex > 0 { - return fmt.Sprintf("%s%d", configName, manifestIndex+1) + // For the case where ConsolidateManifests is true and the manifest's name is provided, + if manifestNameCounter == nil && configName != "" { + return configName + } + + if manifestNameCounter != nil && configName != "" { + *manifestNameCounter++ + if *manifestNameCounter > 1 { + return fmt.Sprintf("%s%d", configName, *manifestNameCounter) } return configName } configName = policyConf.Name - if policyNum > 0 { - return fmt.Sprintf("%s%d", configName, policyNum+1) + + *policyNameCounter++ + if *policyNameCounter > 1 { + return fmt.Sprintf("%s%d", configName, *policyNameCounter) } return configName