fix(policy): policy devel lint --format removes the comments (#2325)

Signed-off-by: Sylwester Piskozub <[email protected]>

Author: Piskoo

app/cli/internal/policydevel/lint.go

@@ -16,6 +16,7 @@
 package policydevel
 
 import (
+	"bytes"
 	"context"
 	"embed"
 	"fmt"
@@ -25,15 +26,14 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/bufbuild/protoyaml-go"
 	v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1"
 	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/unmarshal"
 	"github.com/chainloop-dev/chainloop/pkg/resourceloader"
 	"github.com/open-policy-agent/opa/v1/format"
 	"github.com/styrainc/regal/pkg/config"
 	"github.com/styrainc/regal/pkg/linter"
 	"github.com/styrainc/regal/pkg/rules"
-	"gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v3"
 )
 
 //go:embed .regal.yaml
@@ -204,11 +204,30 @@ func (p *PolicyToLint) validateYAMLFile(file *File) {
 
 	// Update policy file with formatted content
 	if p.Format {
-		outYAML, err := protoyaml.Marshal(&policy)
-		if err != nil {
-			p.AddError(file.Path, fmt.Sprintf("failed to marshal updated YAML: %v", err), 0)
-		} else if err := os.WriteFile(file.Path, outYAML, 0600); err != nil {
-			p.AddError(file.Path, fmt.Sprintf("failed to save updated file: %v", err), 0)
+		var root yaml.Node
+		if err := yaml.Unmarshal(file.Content, &root); err != nil {
+			p.AddError(file.Path, fmt.Sprintf("failed to parse YAML: %v", err), 0)
+			return
+		}
+
+		if err := p.updateEmbeddedRegoInYAML(file, &root); err != nil {
+			p.AddError(file.Path, fmt.Sprintf("failed to update embedded Rego: %v", err), 0)
+			return
+		}
+
+		var buf bytes.Buffer
+		enc := yaml.NewEncoder(&buf)
+		enc.SetIndent(2)
+		defer enc.Close()
+
+		if err := enc.Encode(&root); err != nil {
+			p.AddError(file.Path, fmt.Sprintf("failed to encode YAML: %v", err), 0)
+			return
+		}
+
+		outYAML := buf.Bytes()
+		if err := os.WriteFile(file.Path, outYAML, 0600); err != nil {
+			p.AddError(file.Path, fmt.Sprintf("failed to write updated file: %v", err), 0)
 		} else {
 			file.Content = outYAML
 		}
@@ -404,3 +423,61 @@ func (p *PolicyToLint) processRegalViolation(rawErr error, path string) {
 		p.AddError(path, line, 0)
 	}
 }
+
+// Updates the embedded rego policies in a YAML file
+// Manual update required due to yaml.marshal limitations
+func (p *PolicyToLint) updateEmbeddedRegoInYAML(file *File, rootNode *yaml.Node) error {
+	if rootNode.Kind != yaml.DocumentNode || len(rootNode.Content) == 0 {
+		return fmt.Errorf("unexpected YAML root structure")
+	}
+
+	doc := rootNode.Content[0]
+	if doc.Kind != yaml.MappingNode {
+		return fmt.Errorf("expected mapping node at document root")
+	}
+
+	// Locate spec policy node
+	var specNode *yaml.Node
+	for i := 0; i < len(doc.Content)-1; i += 2 {
+		if doc.Content[i].Value == "spec" && doc.Content[i+1].Kind == yaml.MappingNode {
+			specNode = doc.Content[i+1]
+			break
+		}
+	}
+	if specNode == nil {
+		return fmt.Errorf("spec node not found")
+	}
+
+	// Locate policies node within spec
+	var policiesNode *yaml.Node
+	for i := 0; i < len(specNode.Content)-1; i += 2 {
+		if specNode.Content[i].Value == "policies" && specNode.Content[i+1].Kind == yaml.SequenceNode {
+			policiesNode = specNode.Content[i+1]
+			break
+		}
+	}
+	if policiesNode == nil {
+		return fmt.Errorf("spec.policies node not found")
+	}
+
+	// Iterate over and update each rego policy
+	for _, policy := range policiesNode.Content {
+		if policy.Kind != yaml.MappingNode {
+			continue
+		}
+
+		for i := 0; i < len(policy.Content)-1; i += 2 {
+			key := policy.Content[i]
+			val := policy.Content[i+1]
+
+			if key.Value == "embedded" && val.Kind == yaml.ScalarNode {
+				formatted := p.validateAndFormatRego(val.Value, file.Path)
+				if formatted != val.Value {
+					val.Value = formatted
+				}
+			}
+		}
+	}
+
+	return nil
+}

docs/examples/policies/chainloop-commit.yaml

@@ -21,45 +21,44 @@ spec:
     - kind: ATTESTATION
       embedded: |
         package main
-        
+
         import rego.v1
-                  
+
         ################################
         # Common section do NOT change #
         ################################
-        
+
         result := {
-          "skipped": skipped,
-          "violations": violations,
-          "skip_reason": skip_reason,
+        	"skipped": skipped,
+        	"violations": violations,
+        	"skip_reason": skip_reason,
         }
-          
+
         default skip_reason := ""
-          
+
         skip_reason := m if {
-          not valid_input
-          m := "the file content is not recognized"
+        	not valid_input
+        	m := "the file content is not recognized"
         }
-          
+
         default skipped := true
-        
+
         skipped := false if valid_input
-                  
+
         ########################################
         # EO Common section, custom code below #
         ########################################
-        
+
         # TODO: update to validate if the file is expected, i.e checking the tool that generates it
         valid_input := true
-        
+
         violations contains msg if {
-          not has_commit
-          msg := "missing commit in attestation material"
+        	not has_commit
+        	msg := "missing commit in attestation material"
         }
-        
+
         has_commit if {
-          some sub in input.subject
-          sub.name == "git.head"
-          sub.digest.sha1
+        	some sub in input.subject
+        	sub.name == "git.head"
+        	sub.digest.sha1
         }
-    

docs/examples/policies/chainloop-qa.yaml

@@ -24,45 +24,45 @@ spec:
     - kind: ATTESTATION
       embedded: |
         package main
-        
+
         import rego.v1
-        
+
         ################################
         # Common section do NOT change #
         ################################
-        
+
         result := {
-          "skipped": skipped,
-          "violations": violations,
-          "skip_reason": skip_reason,
+        	"skipped": skipped,
+        	"violations": violations,
+        	"skip_reason": skip_reason,
         }
-        
+
         default skip_reason := ""
-        
+
         skip_reason := m if {
-          not valid_input
-          m := "the file content is not recognized"
+        	not valid_input
+        	m := "the file content is not recognized"
         }
-        
+
         default skipped := true
-        
+
         skipped := false if valid_input
-        
+
         ########################################
         # EO Common section, custom code below #
         ########################################
-        
+
         # TODO: update to validate if the file is expected, i.e checking the tool that generates it
         valid_input := true
-        
+
         violations contains msg if {
-          not is_approved
-          
-          msg:= "Container image is not approved"
+        	not is_approved
+
+        	msg := "Container image is not approved"
         }
-        
+
         is_approved if {
-          input.predicate.annotations.approval == "true"
-          some material in input.predicate.materials
-          material.annotations["chainloop.material.type"] == "CONTAINER_IMAGE"
+        	input.predicate.annotations.approval == "true"
+        	some material in input.predicate.materials
+        	material.annotations["chainloop.material.type"] == "CONTAINER_IMAGE"
         }

docs/examples/policies/quickstart/cdx-fresh.yaml

@@ -1,57 +1,57 @@
 apiVersion: workflowcontract.chainloop.dev/v1
 kind: Policy
 metadata:
-    name: cdx-fresh
-    description: Checks that SBOM is maximum of 30 days old
-    annotations:
-        category: quickstart
+  name: cdx-fresh
+  description: Checks that SBOM is maximum of 30 days old
+  annotations:
+    category: quickstart
 spec:
-    policies:
-        - embedded: |
-            package main
-
-            import rego.v1
-
-            ################################
-            # Common section do NOT change #
-            ################################
-
-            result := {
-            	"skipped": skipped,
-            	"violations": violations,
-            	"skip_reason": skip_reason,
-            	"ignore": ignore,
-            }
-
-            default skip_reason := ""
-
-            skip_reason := m if {
-            	not valid_input
-            	m := "invalid input"
-            }
-
-            default skipped := true
-
-            skipped := false if valid_input
-
-            default ignore := false
-
-            ########################################
-            # EO Common section, custom code below #
-            ########################################
-            # Validates if the input is valid and can be understood by this policy
-            valid_input := true
-
-            limit := 30
-            nanosecs_per_second := (1000 * 1000) * 1000
-            nanosecs_per_day := ((24 * 60) * 60) * nanosecs_per_second
-            maximum_age := limit * nanosecs_per_day
-
-            # If the input is valid, check for any policy violation here
-            violations contains msg if {
-            	sbom_ns = time.parse_rfc3339_ns(input.metadata.timestamp)
-            	exceeding = time.now_ns() - (sbom_ns + maximum_age)
-            	exceeding > 0
-            	msg := sprintf("SBOM created at: %s which is too old (freshness limit set to %d days)", [input.metadata.timestamp, limit])
-            }
-          kind: SBOM_CYCLONEDX_JSON
+  policies:
+    - embedded: |
+        package main
+
+        import rego.v1
+
+        ################################
+        # Common section do NOT change #
+        ################################
+
+        result := {
+        	"skipped": skipped,
+        	"violations": violations,
+        	"skip_reason": skip_reason,
+        	"ignore": ignore,
+        }
+
+        default skip_reason := ""
+
+        skip_reason := m if {
+        	not valid_input
+        	m := "invalid input"
+        }
+
+        default skipped := true
+
+        skipped := false if valid_input
+
+        default ignore := false
+
+        ########################################
+        # EO Common section, custom code below #
+        ########################################
+        # Validates if the input is valid and can be understood by this policy
+        valid_input := true
+
+        limit := 30
+        nanosecs_per_second := (1000 * 1000) * 1000
+        nanosecs_per_day := ((24 * 60) * 60) * nanosecs_per_second
+        maximum_age := limit * nanosecs_per_day
+
+        # If the input is valid, check for any policy violation here
+        violations contains msg if {
+        	sbom_ns = time.parse_rfc3339_ns(input.metadata.timestamp)
+        	exceeding = time.now_ns() - (sbom_ns + maximum_age)
+        	exceeding > 0
+        	msg := sprintf("SBOM created at: %s which is too old (freshness limit set to %d days)", [input.metadata.timestamp, limit])
+        }
+      kind: SBOM_CYCLONEDX_JSON