Skip to content

Commit

Permalink
Skip hashing annotations (#267)
Browse files Browse the repository at this point in the history
* add option to ignore annotations for the purposes of hashing a resource
* ch
  • Loading branch information
ilackarms authored and soloio-bulldozer[bot] committed Sep 10, 2019
1 parent 6f7e0f9 commit 7d0aaf3
Show file tree
Hide file tree
Showing 18 changed files with 727 additions and 79 deletions.
3 changes: 3 additions & 0 deletions api/v1/solo-kit.proto
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ message Resource {
bool cluster_scoped = 3;
// indicates whether documentation generation has to be skipped for the given resource, defaults to false
bool skip_docs_gen = 4;
// indicates whether annotations should be excluded from the resource's generated hash function.
// if set to true, changes in annotations will not cause a new snapshot to be emitted
bool skip_hashing_annotations = 5;
}

extend google.protobuf.FieldOptions {
Expand Down
4 changes: 4 additions & 0 deletions changelog/v0.10.18/skip-annotations.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
changelog:
- type: NEW_FEATURE
description: add option to ignore annotations for the purposes of hashing a resource
issueLink: https://github.com/solo-io/solo-kit/pull/266

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

60 changes: 36 additions & 24 deletions pkg/api/v1/resources/core/solo-kit.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions pkg/code-generator/codegen/templates/resource_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ func (r *{{ .Name }}) Hash() uint64 {
resources.UpdateMetadata(clone, func(meta *core.Metadata) {
meta.ResourceVersion = ""
{{- if $.SkipHashingAnnotations }}
meta.Annotations = nil
{{- end }}
})
return hashutils.HashAll(clone)
Expand All @@ -82,6 +86,9 @@ func (r *{{ .Name }}) SetStatus(status core.Status) {
func (r *{{ .Name }}) Hash() uint64 {
metaCopy := r.GetMetadata()
metaCopy.ResourceVersion = ""
{{- if $.SkipHashingAnnotations }}
metaCopy.Annotations = nil
{{- end }}
return hashutils.HashAll(
metaCopy,
{{- range .Fields }}
Expand Down
12 changes: 7 additions & 5 deletions pkg/code-generator/model/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ type CustomResourceConfig struct {
// the import path for the Go Type
Package string `json:"package"`
// the name of the Go Type
Type string `json:"type"`
PluralName string `json:"plural_name"`
ShortName string `json:"short_name"`
ClusterScoped bool `json:"cluster_scoped"`

Type string `json:"type"`
PluralName string `json:"plural_name"`
ShortName string `json:"short_name"`
ClusterScoped bool `json:"cluster_scoped"`
SkipHashingAnnotations bool `json:"skip_hashing_annotations"`
// set by load
Imported bool
}
Expand Down Expand Up @@ -102,6 +102,8 @@ type Resource struct {
CustomResource CustomResourceConfig // this struct will be empty unless IsCustom is true
CustomImportPrefix string // import prefix for the struct type the generated wrapper will wrap

SkipHashingAnnotations bool // if true, zero out annotations in the generated hash func

Fields []*Field
Oneofs []*Oneof

Expand Down
51 changes: 27 additions & 24 deletions pkg/code-generator/parser/parser_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,16 +97,17 @@ func getResources(project *model.Project, allProjectConfigs []*model.ProjectConf
impPrefix = strings.Replace(impPrefix, ".", "_", -1)
impPrefix = strings.Replace(impPrefix, "-", "_", -1)
resources = append(resources, &model.Resource{
Name: custom.Type,
ShortName: custom.ShortName,
PluralName: custom.PluralName,
GoPackage: custom.Package,
ClusterScoped: custom.ClusterScoped,
CustomImportPrefix: impPrefix,
SkipDocsGen: true,
Project: project,
IsCustom: true,
CustomResource: custom,
Name: custom.Type,
ShortName: custom.ShortName,
PluralName: custom.PluralName,
GoPackage: custom.Package,
ClusterScoped: custom.ClusterScoped,
SkipHashingAnnotations: custom.SkipHashingAnnotations,
CustomImportPrefix: impPrefix,
SkipDocsGen: true,
Project: project,
IsCustom: true,
CustomResource: custom,
})
}

Expand Down Expand Up @@ -193,8 +194,8 @@ func describeResource(messageWrapper ProtoMessageWrapper) (*model.Resource, erro

name := msg.GetName()
var (
shortName, pluralName string
clusterScoped, skipDocsGen bool
shortName, pluralName string
clusterScoped, skipDocsGen, skipHashingAnnotations bool
)
resourceOpts, err := proto.GetExtension(msg.Options, core.E_Resource)
if err != nil {
Expand All @@ -220,6 +221,7 @@ func describeResource(messageWrapper ProtoMessageWrapper) (*model.Resource, erro
pluralName = res.PluralName
clusterScoped = res.ClusterScoped
skipDocsGen = res.SkipDocsGen
skipHashingAnnotations = res.SkipHashingAnnotations
}

// always make it upper camel
Expand All @@ -231,17 +233,18 @@ func describeResource(messageWrapper ProtoMessageWrapper) (*model.Resource, erro
oneofs := collectOneofs(msg)

return &model.Resource{
Name: name,
ProtoPackage: msg.GetPackage(),
GoPackage: messageWrapper.GoPackage,
ShortName: shortName,
PluralName: pluralName,
HasStatus: hasStatus,
Fields: fields,
Oneofs: oneofs,
ClusterScoped: clusterScoped,
SkipDocsGen: skipDocsGen,
Filename: msg.GetFile().GetName(),
Original: msg,
Name: name,
ProtoPackage: msg.GetPackage(),
GoPackage: messageWrapper.GoPackage,
ShortName: shortName,
PluralName: pluralName,
HasStatus: hasStatus,
Fields: fields,
Oneofs: oneofs,
ClusterScoped: clusterScoped,
SkipHashingAnnotations: skipHashingAnnotations,
SkipDocsGen: skipDocsGen,
Filename: msg.GetFile().GetName(),
Original: msg,
}, nil
}
2 changes: 2 additions & 0 deletions pkg/multicluster/v1/multicluster.solo.io_suite_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions test/mocks/api/v2alpha1/mock_resources.proto
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,13 @@ message MockResource {
bool oneof_two = 2;
};
}

message FrequentlyChangingAnnotationsResource {
option (core.solo.io.resource).short_name = "fcar";
option (core.solo.io.resource).plural_name = "fcars";
option (core.solo.io.resource).skip_hashing_annotations = true;

core.solo.io.Metadata metadata = 7 [(gogoproto.nullable) = false];

string blah = 1 ;
}
13 changes: 13 additions & 0 deletions test/mocks/tests/resource_hashing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
. "github.com/onsi/gomega"
"github.com/solo-io/solo-kit/pkg/api/v1/resources/core"
v1 "github.com/solo-io/solo-kit/test/mocks/v1"
"github.com/solo-io/solo-kit/test/mocks/v2alpha1"
)

var _ = Describe("Resource Hashing", func() {
Expand All @@ -17,6 +18,18 @@ var _ = Describe("Resource Hashing", func() {
Expect(originalSnap.Hash()).To(Equal(snapWithInsensitiveChanged.Hash()))
Expect(originalSnap.Hash()).NotTo(Equal(snapWithSensitiveChanged.Hash()))
})
Context("skip_hashing_annotations=true", func() {
It("ignores the resource meta annotations in the hash", func() {
res1 := v2alpha1.NewFrequentlyChangingAnnotationsResource("a", "b")
res2 := v2alpha1.NewFrequentlyChangingAnnotationsResource("a", "b")

// sanity check
Expect(res1.Hash()).To(Equal(res2.Hash()))

res1.Metadata.Annotations = map[string]string{"ignore": "me"}
Expect(res1.Hash()).To(Equal(res2.Hash()))
})
})
})

func snapWithFields(hashSensitiveField, hashInsensitiveField string) *v1.TestingSnapshot {
Expand Down
2 changes: 2 additions & 0 deletions test/mocks/v1/testing.solo.io_suite_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions test/mocks/v1alpha1/testing.solo.io_suite_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 7d0aaf3

Please sign in to comment.