- 
                Notifications
    You must be signed in to change notification settings 
- Fork 34
Fix the bug where raw is abead of obj-temp #175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix the bug where raw is abead of obj-temp #175
Conversation
| @yiraeChristineKim I think this is getting too complicated. How about this alternative implementation? diff --git a/internal/utils.go b/internal/utils.go
index 39b2cd9..92414a5 100644
--- a/internal/utils.go
+++ b/internal/utils.go
@@ -169,11 +169,9 @@ func getPolicyTemplates(policyConf *types.PolicyConfig) ([]map[string]interface{
        objectTemplates := make([]map[string]interface{}, 0, objectTemplatesLength)
        policyTemplates := make([]map[string]interface{}, 0, policyTemplatesLength)
 
-       policyNameCounter := 0
-       // This is used to identify the first `manifests[].name` when `ConsolidateManifests` is set to true.
-       // If `manifests[].name` is not present, the consolidated Configuration policy will
-       // default to using `policies.name`.
-       firstObjTempNameIndex := -1
+       var consolidatedPolicyName string
+
+       policyNameCounter := map[string]int{}
 
        for i, manifestGroup := range manifestGroups {
                complianceType := policyConf.Manifests[i].ComplianceType
@@ -183,7 +181,10 @@ func getPolicyTemplates(policyConf *types.PolicyConfig) ([]map[string]interface{
                ignorePending := policyConf.Manifests[i].IgnorePending
                extraDeps := policyConf.Manifests[i].ExtraDependencies
 
-               manifestNameCounter := 0
+               policyName := policyConf.Manifests[i].Name
+               if policyName == "" {
+                       policyName = policyConf.Name
+               }
 
                for _, manifest := range manifestGroup {
                        err := setGatekeeperEnforcementAction(manifest,
@@ -208,11 +209,13 @@ func getPolicyTemplates(policyConf *types.PolicyConfig) ([]map[string]interface{
 
                                _, found, _ := unstructured.NestedString(manifest, "object-templates-raw")
                                if found {
+                                       policyNameCounter[policyName]++
+
                                        policyTemplate = buildPolicyTemplate(
                                                policyConf,
                                                manifest["object-templates-raw"],
                                                &policyConf.Manifests[i].ConfigurationPolicyOptions,
-                                               getConfigurationPolicyName(policyConf, i, &policyNameCounter, &manifestNameCounter),
+                                               getConfigurationPolicyName(policyName, policyNameCounter[policyName]),
                                        )
                                } else {
                                        policyTemplate = map[string]interface{}{"objectDefinition": manifest}
@@ -260,28 +263,27 @@ func getPolicyTemplates(policyConf *types.PolicyConfig) ([]map[string]interface{
                        }
 
                        if policyConf.ConsolidateManifests {
+                               if consolidatedPolicyName == "" {
+                                       consolidatedPolicyName = policyConf.Manifests[i].Name
+                               }
                                // put all objTemplate with manifest into single consolidated objectTemplates
                                objectTemplates = append(objectTemplates, objTemplate)
                        } else {
                                // casting each objTemplate with manifest to objectTemplates type
                                // build policyTemplate for each objectTemplates
+                               policyNameCounter[policyName]++
+
                                policyTemplate := buildPolicyTemplate(
                                        policyConf,
                                        []map[string]interface{}{objTemplate},
                                        &policyConf.Manifests[i].ConfigurationPolicyOptions,
-                                       getConfigurationPolicyName(policyConf, i,
-                                               &policyNameCounter, &manifestNameCounter),
+                                       getConfigurationPolicyName(policyName, policyNameCounter[policyName]),
                                )
 
                                setTemplateOptions(policyTemplate, ignorePending, extraDeps)
 
                                policyTemplates = append(policyTemplates, policyTemplate)
                        }
-
-                       // If `firstObjTempNameIndex` is already set, skip this step.
-                       if firstObjTempNameIndex == -1 && policyConf.Manifests[i].Name != "" {
-                               firstObjTempNameIndex = i
-                       }
                }
        }
 
@@ -294,13 +296,19 @@ 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 consolidatedPolicyName == "" {
+                       consolidatedPolicyName = policyConf.Name
+               }
+
+               policyNameCounter[consolidatedPolicyName]++
+
                // 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, firstObjTempNameIndex, &policyNameCounter, nil),
+                       getConfigurationPolicyName(consolidatedPolicyName, policyNameCounter[consolidatedPolicyName]),
                )
                setTemplateOptions(policyTemplate, policyConf.IgnorePending, policyConf.ExtraDependencies)
                policyTemplates = append(policyTemplates, policyTemplate)
@@ -336,42 +344,12 @@ func getPolicyTemplates(policyConf *types.PolicyConfig) ([]map[string]interface{
        return policyTemplates, nil
 }
 
-// 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 {
-       var configName string
-
-       if manifestGroupIndex >= 0 {
-               // Do not append a number to configName when it is used for the first time.
-               configName = policyConf.Manifests[manifestGroupIndex].Name
-
-               // 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
-
-       *policyNameCounter++
-       if *policyNameCounter > 1 {
-               return fmt.Sprintf("%s%d", configName, *policyNameCounter)
+func getConfigurationPolicyName(name string, count int) string {
+       if count > 1 {
+               return fmt.Sprintf("%s%d", name, count)
        }
 
-       return configName
+       return name
 }
 
 // setGatekeeperEnforcementAction function override gatekeeper.constraint.enforcementAction | 
9e0d6f6    to
    81105b1      
    Compare
  
    | @mprahl   | 
When raw-template manifest is ahead of obj-temp manifest, the numbering of policyName doesn't work properly Signed-off-by: yiraeChristineKim <[email protected]>
81105b1    to
    0fe7744      
    Compare
  
    | [APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mprahl, yiraeChristineKim The full list of commands accepted by this bot can be found here. The pull request process is described here 
Needs approval from an approver in each of these files:
 
 Approvers can indicate their approval by writing  | 
There is a bug when
raw-templatemanifest is ahead ofobj-templatemanifest.https://issues.redhat.com/browse/ACM-12563