diff --git a/go.mod b/go.mod index fef821b..658a5af 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 413cd4f..81e5b33 100644 --- a/go.sum +++ b/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= diff --git a/internal/crdschema/crd.go b/internal/crdschema/crd.go index 3372674..916f626 100644 --- a/internal/crdschema/crd.go +++ b/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) { diff --git a/internal/crdschema/crd_test.go b/internal/crdschema/crd_test.go index 1ffa6b8..7d1ff71 100644 --- a/internal/crdschema/crd_test.go +++ b/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