Add tests for breaking changes detection in CRD self-versions

Signed-off-by: Alper Rifat Ulucinar <[email protected]>

go.mod

@@ -36,6 +36,7 @@ require (
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 // indirect
 	github.com/yuin/goldmark v1.5.3 // indirect
+	golang.org/x/mod v0.7.0 // indirect
 	golang.org/x/net v0.0.0-20221014081412-f15817d10f9b // indirect
 	golang.org/x/oauth2 v0.0.0-20221014153046-6fdb5e3db783 // indirect
 	golang.org/x/sys v0.0.0-20220728004956-3c1f35247d10 // indirect

go.sum

@@ -548,6 +548,8 @@ golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
+golang.org/x/mod v0.7.0 h1:LapD9S96VoQRhi/GrNTqeBJFrUjs5UHCAtTlgwA5oZA=
+golang.org/x/mod v0.7.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=

internal/crdschema/crd.go

@@ -22,6 +22,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/tufin/oasdiff/diff"
 	"github.com/tufin/oasdiff/report"
+	"golang.org/x/mod/semver"
 	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiyaml "k8s.io/apimachinery/pkg/util/yaml"
 	k8syaml "sigs.k8s.io/yaml"
@@ -33,6 +34,8 @@ const (
 	errBreakingSelfVersionsCompute    = "failed to compute breaking changes in the versions of a CRD"
 )
 
+// SchemaCheck represents a schema checker that can return the set of breaking
+// API changes between schemas.
 type SchemaCheck interface {
 	GetBreakingChanges() (map[string]*diff.Diff, error)
 }
@@ -144,6 +147,7 @@ func (d *SelfDiff) GetBreakingChanges() (map[string]*diff.Diff, error) {
 	if len(selfDocs) < 2 {
 		return diffMap, nil
 	}
+	sortVersions(selfDocs)
 	prev := 0
 	for prev < len(selfDocs)-1 {
 		revisionDoc := selfDocs[prev+1]
@@ -152,10 +156,30 @@ func (d *SelfDiff) GetBreakingChanges() (map[string]*diff.Diff, error) {
 			return nil, errors.Wrap(err, errBreakingSelfVersionsCompute)
 		}
 		diffMap[revisionDoc.Info.Version] = sd
+		prev = prev + 1
 	}
 	return diffMap, nil
 }
 
+func sortVersions(versions []*openapi3.T) {
+	versionNames := make([]string, 0, len(versions))
+	for _, t := range versions {
+		versionNames = append(versionNames, t.Info.Version)
+	}
+	semver.Sort(versionNames)
+	for i, v := range versionNames {
+		for j := range versions {
+			if versions[j].Info.Version != v {
+				continue
+			}
+			tmp := versions[i]
+			versions[i] = versions[j]
+			versions[j] = tmp
+			break
+		}
+	}
+}
+
 // GetBreakingChanges returns a diff representing
 // the detected breaking schema changes between the base and revision CRDs.
 func (d *RevisionDiff) GetBreakingChanges() (map[string]*diff.Diff, error) {

internal/crdschema/crd_test.go

@@ -246,9 +246,10 @@ func Test_GetRevisionBreakingChanges(t *testing.T) {
 			},
 		},
 	}
+
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
-			diff, err := newDiffWithModifiers(tt.args.basePath, tt.args.basePath, tt.args.revisionModifiers...)
+			diff, err := newRevisionDiffWithModifiers(tt.args.basePath, tt.args.basePath, tt.args.revisionModifiers...)
 			if err != nil {
 				t.Errorf("\n%s\nnewDiffWithModifiers(...): failed to load base or revision CRD:\n%v", tt.reason, err)
 				return
@@ -276,9 +277,96 @@ func Test_GetRevisionBreakingChanges(t *testing.T) {
 	}
 }
 
+func Test_GetSelfBreakingChanges(t *testing.T) {
+	type want struct {
+		errExpected     bool
+		breakingChanges map[int]string
+	}
+	type args struct {
+		crdPath           string
+		revisionModifiers []crdModifier
+	}
+	tests := map[string]struct {
+		reason string
+		args   args
+		want   want
+	}{
+		"NoBreakingChanges": {
+			reason: "No Diff should be reported if the version schemas are identical",
+			args: args{
+				crdPath: "testdata/base.yaml",
+			},
+		},
+		"BreakingChangeInV1Beta2": {
+			reason: "Changing the type of an existing field is a breaking API change in v1beta2",
+			args: args{
+				crdPath: "testdata/base.yaml",
+				revisionModifiers: []crdModifier{
+					func(r *v1.CustomResourceDefinition) {
+						p := getSpecForProviderProperty(r, 1, "certificateChain")
+						p.Type = "int"
+						addSpecForProviderProperty(r, 1, "certificateChain", p, nil)
+					},
+				},
+			},
+			want: want{
+				breakingChanges: map[int]string{1: `
+- Schema changed
+  - Properties changed
+    - Modified property: spec
+      - Properties changed
+        - Modified property: forProvider
+          - Properties changed
+            - Modified property: certificateChain
+              - Type changed from 'string' to 'int'`},
+			},
+		},
+	}
+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			diff, err := newSelfDiffWithModifiers(tt.args.crdPath, tt.args.revisionModifiers...)
+			if err != nil {
+				t.Errorf("\n%s\nnewDiffWithModifiers(...): failed to load the CRD:\n%v", tt.reason, err)
+				return
+			}
+			m, err := diff.GetBreakingChanges()
+			if (err != nil) != tt.want.errExpected {
+				t.Errorf("\n%s\nGetBreakingChanges(): error = %v, wantErr = %v", tt.reason, err, tt.want.errExpected)
+				return
+			}
+			if err != nil {
+				return
+			}
+			for i, d := range tt.want.breakingChanges {
+				version := diff.crd.Spec.Versions[i].Name
+				if (len(d) == 0) != m[version].Empty() {
+					t.Errorf("\n%s\nGetBreakingChanges(): (len(breakingChanges) == 0) = %v, isEmpty = %v, diff = \n%s", tt.reason, len(d) == 0, m[version].Empty(), GetDiffReport(m[version]))
+					return
+				}
+				got := GetDiffReport(m[version])
+				if diff := cmp.Diff(strings.TrimSpace(d), strings.TrimSpace(got)); diff != "" {
+					t.Errorf("\n%s\nGetDiffReport(...): -want, +got:\n%s", tt.reason, diff)
+				}
+			}
+		})
+	}
+}
+
 type crdModifier func(crd *v1.CustomResourceDefinition)
 
-func newDiffWithModifiers(basePath, revisionPath string, revisionModifiers ...crdModifier) (*RevisionDiff, error) {
+func newSelfDiffWithModifiers(crdPath string, crdModifiers ...crdModifier) (*SelfDiff, error) {
+	d, err := NewSelfDiff(crdPath)
+	if err != nil {
+		return nil, err
+	}
+	for _, m := range crdModifiers {
+		m(d.crd)
+	}
+	return d, nil
+}
+
+func newRevisionDiffWithModifiers(basePath, revisionPath string, revisionModifiers ...crdModifier) (*RevisionDiff, error) {
 	d, err := NewRevisionDiff(basePath, revisionPath)
 	if err != nil {
 		return nil, err