Skip to content

Commit a7a5970

Browse files
committed
diff go structs on schema changes
hack/check-schema-changes.sh checks whether the PR compared to master contains any changes in the config.go files under pkg/skaffold/schema. If yes, it checks if those changes are structural changes or not. If they are and the package is not "latest", then we'll fail the PR as we assume anything else than latest is already released and shouldn't be changed. If the change is latest and it is released, we fail the PR for the same reason. If the change is in latest and it is not released yet, it is fine to make changes.
1 parent 43e326a commit a7a5970

File tree

9 files changed

+596
-35
lines changed

9 files changed

+596
-35
lines changed

hack/check-schema-changes.sh

+70
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
#!/usr/bin/env bash
2+
3+
# Copyright 2019 The Skaffold Authors
4+
#
5+
# Licensed under the Apache License, Version 2.0 (the "License");
6+
# you may not use this file except in compliance with the License.
7+
# You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
18+
# This check checks whether the PR compared to master contains any changes
19+
# in the config.go files under pkg/skaffold/schema. If yes, it checks if those changes
20+
# are structural changes or not.
21+
# If they are structural changes and the package is not "latest",
22+
# then we'll fail the PR as we assume anything else than latest is already released and
23+
# shouldn't be changed.
24+
# If the change is latest and it is released, we fail the PR for the same reason.
25+
# If the change is in latest and it is not released yet, it is fine to make changes.
26+
27+
28+
function changeDetected() {
29+
echo "--------"
30+
echo "Structural change detected in a released config: $1"
31+
echo "Please create a new PR first with a new version."
32+
echo "You can use 'hack/new_version.sh' to generate the new config version."
33+
echo "Admin rights are required to merge this PR!"
34+
echo "--------"
35+
git diff master -- $1
36+
}
37+
38+
set +x
39+
CHANGED_CONFIG_FILES=$(git diff --name-only master -- pkg/skaffold/schema | grep config.go)
40+
41+
if [[ -z "${CHANGED_CONFIG_FILES}" ]]; then
42+
exit 0
43+
fi
44+
45+
result=0
46+
47+
for f in ${CHANGED_CONFIG_FILES}
48+
do
49+
cat ${f} > /tmp/a.go
50+
git show master:${f} > /tmp/b.go
51+
go run hack/versions/cmd/diff/diff.go -- /tmp/a.go /tmp/b.go > /dev/null 2>&1
52+
status=$?
53+
if [[ ${status} -ne 0 ]]; then
54+
# changes in latest
55+
if [[ "${f}" == *"latest"* ]]; then
56+
echo "structural changes in latest config, checking on Github if latest is released..."
57+
latest=$(go run hack/versions/cmd/latest/latest.go)
58+
echo ${latest}
59+
if [[ "${latest}" == *"is released"* ]]; then
60+
changeDetected ${f}
61+
result=1
62+
fi
63+
else
64+
changeDetected ${f}
65+
result=1
66+
fi
67+
fi
68+
done
69+
70+
exit $result

hack/checks.sh

+2-1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ RESET='\033[0m'
2020

2121
echo "Running validation scripts..."
2222
scripts=(
23+
"hack/check-schema-changes.sh"
2324
"hack/boilerplate.sh"
2425
"hack/gofmt.sh"
2526
"hack/linter.sh"
@@ -41,4 +42,4 @@ for s in "${scripts[@]}"; do
4142
fail=1
4243
fi
4344
done
44-
exit $fail
45+
exit $fail

hack/new_version.sh

+2-2
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
set -e
1818

19-
go run ./hack/new_config_version/version.go $@
19+
go run ./hack/versions/cmd/new/version.go $@
2020

2121
goimports -w ./pkg/skaffold/schema
2222
make generate-schemas
@@ -26,7 +26,7 @@ make test
2626
echo
2727
echo "---------------------------------------"
2828
echo
29-
echo "Files generated for $NEW_VERSION."
29+
echo "Files generated for the new version."
3030
echo "All tests should have passed. For the docs change, commit the results and rerun 'make test'."
3131
echo "Please double check manually the generated files as well: the upgrade functionality, and all the examples:"
3232
echo

hack/versions/cmd/diff/diff.go

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
Copyright 2019 The Skaffold Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package main
18+
19+
import (
20+
"fmt"
21+
"os"
22+
23+
"github.com/GoogleContainerTools/skaffold/hack/versions/pkg/diff"
24+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
25+
)
26+
27+
func main() {
28+
if len(os.Args) != 4 {
29+
color.Red.Fprintf(os.Stdout, "ran with args: %v\nUsage: go run diff.go -- [file1] [file2]\n", os.Args)
30+
os.Exit(1)
31+
}
32+
a := os.Args[2]
33+
b := os.Args[3]
34+
d, err := diff.CompareGoStructs(a, b)
35+
if err != nil {
36+
panic(err)
37+
}
38+
if d != "" {
39+
fmt.Printf("there is structural difference between structs of %s and %s\n%s", a, b, d)
40+
os.Exit(1)
41+
}
42+
}

hack/versions/cmd/latest/latest.go

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
Copyright 2019 The Skaffold Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package main
18+
19+
import (
20+
"os"
21+
22+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
23+
24+
"github.com/sirupsen/logrus"
25+
26+
version "github.com/GoogleContainerTools/skaffold/hack/versions/pkg"
27+
)
28+
29+
func main() {
30+
logrus.SetLevel(logrus.ErrorLevel)
31+
latestVersion, isReleased := version.GetLatestVersion()
32+
if isReleased {
33+
color.Red.Fprintf(os.Stdout, "%s is released, it should NOT be changed!\n", latestVersion)
34+
} else {
35+
color.Green.Fprintf(os.Stdout, "%s is unreleased, it is safe to change it.\n", latestVersion)
36+
}
37+
38+
}

hack/new_config_version/version.go renamed to hack/versions/cmd/new/new.go

+6-32
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,17 @@ package main
1818

1919
import (
2020
"bufio"
21-
"context"
22-
"fmt"
2321
"io/ioutil"
24-
"net/http"
2522
"os"
2623
"path/filepath"
2724
"regexp"
2825
"strings"
2926

30-
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
27+
version "github.com/GoogleContainerTools/skaffold/hack/versions/pkg"
3128

3229
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
3330

3431
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema"
35-
"github.com/google/go-github/github"
3632
"github.com/sirupsen/logrus"
3733
)
3834

@@ -42,14 +38,10 @@ func main() {
4238
logrus.SetLevel(logrus.DebugLevel)
4339
prev := strings.TrimPrefix(schema.SchemaVersions[len(schema.SchemaVersions)-2].APIVersion, "skaffold/")
4440
logrus.Infof("Previous Skaffold version: %s", prev)
45-
current := strings.TrimPrefix(latest.Version, "skaffold/")
46-
logrus.Infof("Current Skaffold version: %s", current)
47-
next := readNextVersion()
4841

49-
logrus.Infof("checking for released status of %s...", prev)
50-
lastReleased := getLastReleasedConfigVersion()
51-
logrus.Infof("last released version: %s", lastReleased)
52-
if lastReleased != current {
42+
current, latestIsReleased := version.GetLatestVersion()
43+
44+
if !latestIsReleased {
5345
logrus.Fatalf("There is no need to create a new version, %s is still not released", current)
5446
}
5547

@@ -62,6 +54,8 @@ func main() {
6254
}
6355
})
6456

57+
next := readNextVersion()
58+
6559
// Create code to upgrade from current to new
6660
cp(path(prev, "upgrade.go"), path(current, "upgrade.go"))
6761
sed(path(current, "upgrade.go"), current, next)
@@ -101,26 +95,6 @@ func main() {
10195
sed("docs/config.toml", current, next)
10296
}
10397

104-
func getLastReleasedConfigVersion() string {
105-
client := github.NewClient(nil)
106-
releases, _, _ := client.Repositories.ListReleases(context.Background(), "GoogleContainerTools", "skaffold", &github.ListOptions{})
107-
lastTag := *releases[0].TagName
108-
logrus.Infof("last release tag: %s", lastTag)
109-
configURL := fmt.Sprintf("https://raw.githubusercontent.com/GoogleContainerTools/skaffold/%s/pkg/skaffold/schema/latest/config.go", lastTag)
110-
resp, err := http.Get(configURL)
111-
if err != nil {
112-
logrus.Fatalf("can't determine latest released config version, failed to download %s: %s", configURL, err)
113-
}
114-
defer resp.Body.Close()
115-
config, err := ioutil.ReadAll(resp.Body)
116-
if err != nil {
117-
logrus.Fatalf("failed to read during download %s, err: %s", configURL, err)
118-
}
119-
versionPattern := regexp.MustCompile("const Version string = \"skaffold/(.*)\"")
120-
lastReleased := versionPattern.FindStringSubmatch(string(config))[1]
121-
return lastReleased
122-
}
123-
12498
func makeSchemaDir(new string) {
12599
latestDir, _ := os.Stat(path("latest"))
126100
newDirPath := path(new)

hack/versions/pkg/diff/godiff.go

+118
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/*
2+
Copyright 2019 The Skaffold Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package diff
18+
19+
import (
20+
"fmt"
21+
"go/ast"
22+
"go/parser"
23+
"go/token"
24+
"sort"
25+
"strings"
26+
27+
"github.com/sirupsen/logrus"
28+
29+
"github.com/google/go-cmp/cmp"
30+
)
31+
32+
// CompareGoStructs returns an empty string iff aFile and bFile are valid go files
33+
// and the top level go struct declarations have the same fields independent of order.
34+
// It returns a semi-readable diff of the AST in case the two files are different.
35+
// Returns error when either of the go files are not parseable.
36+
func CompareGoStructs(aFile string, bFile string) (string, error) {
37+
fset := token.NewFileSet()
38+
astA, err := parser.ParseFile(fset, aFile, nil, parser.AllErrors)
39+
if err != nil {
40+
return "", err
41+
}
42+
astB, err := parser.ParseFile(fset, bFile, nil, parser.AllErrors)
43+
if err != nil {
44+
return "", err
45+
}
46+
47+
return cmp.Diff(structsMap(astA), structsMap(astB)), nil
48+
}
49+
50+
func structsMap(astB *ast.File) map[string]string {
51+
bStructs := make(map[string]string)
52+
for _, n := range astB.Decls {
53+
decl, ok := n.(*ast.GenDecl)
54+
if !ok {
55+
continue
56+
}
57+
typeSec, ok := decl.Specs[0].(*ast.TypeSpec)
58+
if !ok {
59+
continue
60+
}
61+
structType, ok := typeSec.Type.(*ast.StructType)
62+
if !ok {
63+
continue
64+
}
65+
bStructs[typeSec.Name.Name] = fieldListString(structType)
66+
}
67+
logrus.Debugf("%+v", bStructs)
68+
return bStructs
69+
}
70+
71+
func fieldListString(structType *ast.StructType) string {
72+
fieldListString := ""
73+
fields := structType.Fields.List
74+
sort.Slice(fields, func(i, j int) bool {
75+
if len(fields[i].Names) == 0 {
76+
return false
77+
}
78+
if len(fields[i].Names) > 0 && len(fields[j].Names) == 0 {
79+
return true
80+
}
81+
return strings.Compare(fields[i].Names[0].Name, fields[j].Names[0].Name) > 0
82+
})
83+
for _, field := range fields {
84+
tag := "'"
85+
if field.Tag != nil {
86+
tag = field.Tag.Value
87+
}
88+
fieldListString = fmt.Sprintf("%+v %s type: %s tag: %v",
89+
fieldListString,
90+
field.Names,
91+
baseTypeName(field.Type),
92+
tag)
93+
}
94+
return fieldListString
95+
}
96+
97+
// inspired by https://github.com/golang/go/blob/9b968df17782f21cc0af14c9d3c0bcf4cf3f911f/src/go/doc/reader.go#L100
98+
func baseTypeName(x ast.Expr) (name string) {
99+
switch t := x.(type) {
100+
case *ast.Ident:
101+
return t.Name
102+
case *ast.SelectorExpr:
103+
if _, ok := t.X.(*ast.Ident); ok {
104+
return t.Sel.Name
105+
}
106+
case *ast.ArrayType:
107+
return "[]" + baseTypeName(t.Elt)
108+
case *ast.MapType:
109+
return "map[" + baseTypeName(t.Key) + "]" + baseTypeName(t.Value)
110+
case *ast.ParenExpr:
111+
return baseTypeName(t.X)
112+
case *ast.StarExpr:
113+
return baseTypeName(t.X)
114+
default:
115+
panic(fmt.Errorf("not covered %+v %+v ", t, x))
116+
}
117+
return
118+
}

0 commit comments

Comments
 (0)