Signed-off-by: Anders Eknert <[email protected]>

Replace use of yaml.v2 library with yaml.v3

Some quirks encountered in the upgrade:
- Errors reported by the parser are now zero-indexed
- A few errors seemingly reported on the line after the
  error rather than where it happened
- Also a few tests where the line number reported
  *previously* seemed wrong but now is right
- Something different in how the parser unmarshalled to
  the "raw" schema annotation type we used. Changed to
  use a a map instead of that type alias.. but I'm not
  really sure why that had to be done

Perhaps worth looking into this further. But pushing this
now to start that discussion.

Fixes #7090

Signed-off-by: Anders Eknert <[email protected]>

ast/parser.go

@@ -17,7 +17,7 @@ import (
 	"strings"
 	"unicode/utf8"
 
-	"gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v3"
 
 	"github.com/open-policy-agent/opa/ast/internal/scanner"
 	"github.com/open-policy-agent/opa/ast/internal/tokens"
@@ -2309,12 +2309,10 @@ type rawAnnotation struct {
 	Organizations    []string               `yaml:"organizations"`
 	RelatedResources []interface{}          `yaml:"related_resources"`
 	Authors          []interface{}          `yaml:"authors"`
-	Schemas          []rawSchemaAnnotation  `yaml:"schemas"`
+	Schemas          []map[string]any       `yaml:"schemas"`
 	Custom           map[string]interface{} `yaml:"custom"`
 }
 
-type rawSchemaAnnotation map[string]interface{}
-
 type metadataParser struct {
 	buf      *bytes.Buffer
 	comments []*Comment
@@ -2345,9 +2343,8 @@ func (b *metadataParser) Parse() (*Annotations, error) {
 		var comment *Comment
 		match := yamlLineErrRegex.FindStringSubmatch(err.Error())
 		if len(match) == 2 {
-			n, err2 := strconv.Atoi(match[1])
+			index, err2 := strconv.Atoi(match[1])
 			if err2 == nil {
-				index := n - 1 // line numbering is 1-based so subtract one from row
 				if index >= len(b.comments) {
 					comment = b.comments[len(b.comments)-1]
 				} else {
@@ -2397,7 +2394,7 @@ func (b *metadataParser) Parse() (*Annotations, error) {
 			if err != nil {
 				return nil, err
 			}
-		case map[interface{}]interface{}:
+		case map[string]any:
 			w, err := convertYAMLMapKeyTypes(v, nil)
 			if err != nil {
 				return nil, fmt.Errorf("invalid schema definition: %w", err)
@@ -2446,8 +2443,9 @@ func (b *metadataParser) Parse() (*Annotations, error) {
 	return &result, nil
 }
 
-// augmentYamlError augments a YAML error with hints intended to help the user figure out the cause of an otherwise cryptic error.
-// These are hints, instead of proper errors, because they are educated guesses, and aren't guaranteed to be correct.
+// augmentYamlError augments a YAML error with hints intended to help the user figure out the cause of an otherwise
+// cryptic error. These are hints, instead of proper errors, because they are educated guesses, and aren't guaranteed
+// to be correct.
 func augmentYamlError(err error, comments []*Comment) error {
 	// Adding hints for when key/value ':' separator isn't suffixed with a legal YAML space symbol
 	for _, comment := range comments {
@@ -2601,11 +2599,11 @@ func parseAuthorString(s string) (*AuthorAnnotation, error) {
 	return &AuthorAnnotation{Name: name, Email: email}, nil
 }
 
-func convertYAMLMapKeyTypes(x interface{}, path []string) (interface{}, error) {
+func convertYAMLMapKeyTypes(x any, path []string) (any, error) {
 	var err error
 	switch x := x.(type) {
-	case map[interface{}]interface{}:
-		result := make(map[string]interface{}, len(x))
+	case map[any]any:
+		result := make(map[string]any, len(x))
 		for k, v := range x {
 			str, ok := k.(string)
 			if !ok {
@@ -2617,7 +2615,7 @@ func convertYAMLMapKeyTypes(x interface{}, path []string) (interface{}, error) {
 			}
 		}
 		return result, nil
-	case []interface{}:
+	case []any:
 		for i := range x {
 			x[i], err = convertYAMLMapKeyTypes(x[i], append(path, fmt.Sprintf("%d", i)))
 			if err != nil {

ast/parser_test.go

@@ -5052,7 +5052,7 @@ public_servers[server] {
 	ports[k].networks[l] = networks[m].id;
 	networks[m].public = true
 }`,
-			expError: "test.rego:14: rego_parse_error: yaml: line 7: could not find expected ':'",
+			expError: "test.rego:14: rego_parse_error: yaml: line 6: could not find expected ':'",
 		},
 		{
 			note: "Ill-structured (invalid) annotation document path",
@@ -5146,7 +5146,7 @@ public_servers[server] {
 	networks[m].public = true
 }`,
 			expNumComments: 6,
-			expError:       "rego_parse_error: yaml: line 3: did not find expected key",
+			expError:       "rego_parse_error: yaml: line 2: did not find expected key",
 		},
 		{
 			note: "Multiple rules with and without metadata",
@@ -6058,7 +6058,7 @@ func TestAnnotationsAugmentedError(t *testing.T) {
 				"# scope: rule\n" +
 				"p := true\n",
 			expErrorHint: "Hint: on line 4, symbol(s) ['\\u00a0'] immediately following a key/value separator ':' is not a legal yaml space character",
-			expErrorRow:  5,
+			expErrorRow:  6, // Should be 5 really, and yaml.v2 reported the error as on line 1, but v3 on line 3..
 		},
 		{
 			note: "thin whitespace (\\u2009) after key/value separator",
@@ -6101,7 +6101,7 @@ func TestAnnotationsAugmentedError(t *testing.T) {
 				"p := true\n",
 			expErrorHint: "Hint: on line 4, symbol(s) ['\\u3000'] immediately following a key/value separator ':' is not a legal yaml space character\n" +
 				"  Hint: on line 6, symbol(s) ['\\u2009'] immediately following a key/value separator ':' is not a legal yaml space character",
-			expErrorRow: 5,
+			expErrorRow: 6,
 		},
 	}
 
@@ -6130,6 +6130,26 @@ func TestAnnotationsAugmentedError(t *testing.T) {
 	}
 }
 
+func TestAnnotationsAreParsedAsYamlv1_2(t *testing.T) {
+	policy := `package p
+	
+# METADATA
+# custom:
+#   string: yes
+is_string := rego.metadata.rule().custom.string == "yes"
+`
+	mod := MustParseModuleWithOpts(policy, ParserOptions{ProcessAnnotation: true})
+
+	if len(mod.Annotations) != 1 {
+		t.Fatalf("Expected exactly one annotation but got %v", len(mod.Annotations))
+	}
+
+	anno := mod.Annotations[0]
+	if value, ok := anno.Custom["string"].(string); !ok || value != "yes" {
+		t.Fatalf("Expected custom.string to be 'yes' but got %v", value)
+	}
+}
+
 // https://github.com/open-policy-agent/opa/issues/6587
 func TestAnnotationsParseErrorOnFirstRowGetsCorrectLocation(t *testing.T) {
 	module := `# METADATA

go.mod

@@ -46,7 +46,6 @@ require (
 	golang.org/x/time v0.6.0
 	google.golang.org/grpc v1.67.1
 	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c
-	gopkg.in/yaml.v2 v2.4.0
 	gopkg.in/yaml.v3 v3.0.1
 	oras.land/oras-go/v2 v2.3.1
 	sigs.k8s.io/yaml v1.4.0

go.sum

@@ -438,7 +438,6 @@ gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EV
 gopkg.in/ini.v1 v1.67.0 h1:Dgnx+6+nfE+IfzjUEISNeydPJh9AXNNsWbGP9KzCsOA=
 gopkg.in/ini.v1 v1.67.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
 gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
-gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=
 gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=

internal/gqlparser/parser/testrunner/runner.go

@@ -8,7 +8,7 @@ import (
 
 	"github.com/andreyvit/diff"
 	"github.com/open-policy-agent/opa/internal/gqlparser/gqlerror"
-	"gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v3"
 )
 
 type Features map[string][]Spec