diff --git a/.golangci.yaml b/.golangci.yaml index f0cf491..cabddd8 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -7,7 +7,6 @@ linters: enable: - "bidichk" - "bodyclose" - - "deadcode" - "errcheck" - "errname" - "errorlint" @@ -17,7 +16,6 @@ linters: - "gosec" - "gosimple" - "govet" - - "ifshort" - "importas" - "ineffassign" - "makezero" @@ -27,12 +25,10 @@ linters: - "revive" - "rowserrcheck" - "staticcheck" - - "structcheck" - "stylecheck" - "tenv" - "typecheck" - "unconvert" - "unused" - - "varcheck" - "wastedassign" - "whitespace" diff --git a/component/ensure_component.go b/component/ensure_component.go index ba52909..ad5ff6a 100644 --- a/component/ensure_component.go +++ b/component/ensure_component.go @@ -55,11 +55,7 @@ func (e *EnsureComponentByHash[K, A]) Handle(ctx context.Context) { ownedObjs := e.List(ctx, e.nn.MustValue(ctx)) newObj := e.newObj(ctx) - hash, err := e.Hash(newObj) - if err != nil { - e.ctrls.RequeueErr(ctx, err) - return - } + hash := e.Hash(newObj) newObj = newObj.WithAnnotations(map[string]string{e.HashAnnotationKey: hash}) matchingObjs := make([]K, 0) @@ -78,7 +74,7 @@ func (e *EnsureComponentByHash[K, A]) Handle(ctx context.Context) { if len(matchingObjs) == 0 { // apply if no matching KubeObject in cluster - _, err = e.applyObject(ctx, newObj) + _, err := e.applyObject(ctx, newObj) if err != nil { e.ctrls.RequeueErr(ctx, err) return diff --git a/hash/hash.go b/hash/hash.go index 6ad1e56..997c032 100644 --- a/hash/hash.go +++ b/hash/hash.go @@ -13,13 +13,13 @@ import ( ) type ( - ObjectHashFunc func(obj any) (string, error) + ObjectHashFunc func(obj any) string EqualFunc func(a, b string) bool ) // ObjectHasher hashes and object and can compare hashes for equality type ObjectHasher interface { - Hash(obj any) (string, error) + Hash(obj any) string Equal(a, b string) bool } @@ -28,7 +28,7 @@ type hasher struct { EqualFunc } -func (h *hasher) Hash(obj interface{}) (string, error) { +func (h *hasher) Hash(obj interface{}) string { return h.ObjectHashFunc(obj) } @@ -54,7 +54,7 @@ func NewObjectHash() ObjectHasher { // SecureObject canonicalizes the object before hashing with sha512 and then // with xxhash -func SecureObject(obj interface{}) (string, error) { +func SecureObject(obj interface{}) string { hasher := sha512.New512_256() printer := spew.ConfigState{ Indent: " ", @@ -62,17 +62,16 @@ func SecureObject(obj interface{}) (string, error) { DisableMethods: true, SpewKeys: true, } - _, err := printer.Fprintf(hasher, "%#v", obj) - if err != nil { - return "", err - } + // sha512's hasher.Write never returns an error, and Fprintf just passes up + // the underlying Write call's error, so we can safely ignore the error here + _, _ = printer.Fprintf(hasher, "%#v", obj) // xxhash the sha512 hash to get a shorter value xxhasher := xxhash.New() - _, err = xxhasher.Write(hasher.Sum(nil)) - if err != nil { - return "", err - } - return rand.SafeEncodeString(fmt.Sprint(xxhasher.Sum(nil))), nil + + // xxhash's hasher.Write never returns an error, so we can safely ignore + // the error here tpp + _, _ = xxhasher.Write(hasher.Sum(nil)) + return rand.SafeEncodeString(fmt.Sprint(xxhasher.Sum(nil))) } // SecureEqual compares hashes safely @@ -81,7 +80,7 @@ func SecureEqual(a, b string) bool { } // Object canonicalizes the object before hashing with xxhash -func Object(obj interface{}) (string, error) { +func Object(obj interface{}) string { hasher := xxhash.New() printer := spew.ConfigState{ Indent: " ", @@ -89,11 +88,11 @@ func Object(obj interface{}) (string, error) { DisableMethods: true, SpewKeys: true, } - _, err := printer.Fprintf(hasher, "%#v", obj) - if err != nil { - return "", err - } - return rand.SafeEncodeString(fmt.Sprint(hasher.Sum(nil))), nil + + // xxhash's hasher.Write never returns an error, and Fprintf just passes up + // the underlying Write call's error, so we can safely ignore the error here + _, _ = printer.Fprintf(hasher, "%#v", obj) + return rand.SafeEncodeString(fmt.Sprint(hasher.Sum(nil))) } // Equal compares hashes safely diff --git a/hash/hash_test.go b/hash/hash_test.go index 77f223e..36a20c5 100644 --- a/hash/hash_test.go +++ b/hash/hash_test.go @@ -12,7 +12,7 @@ func ExampleObject() { "some": "data", }, } - hash, _ := Object(configmap) + hash := Object(configmap) fmt.Println(Equal(hash, "n688h54h56ch64bh677h55fh648hddq")) // Output: true } @@ -23,7 +23,7 @@ func ExampleSecureObject() { "some": "data", }, } - hash, _ := SecureObject(secret) + hash := SecureObject(secret) fmt.Println(SecureEqual(hash, "n665hb8h667h68hfbhffh669h54dq")) // Output: true } diff --git a/hash/set.go b/hash/set.go new file mode 100644 index 0000000..7a38c1f --- /dev/null +++ b/hash/set.go @@ -0,0 +1,107 @@ +package hash + +// Set is a set that stores objects based on their Object hash +type Set[T any] map[string]T + +// NewSet creates a set from a list of objects +func NewSet[T any](objects ...T) Set[T] { + set := make(map[string]T, len(objects)) + for _, o := range objects { + set[Object(o)] = o + } + return set +} + +// Has returns true if all objects are already in the set +func (s Set[T]) Has(objects ...T) bool { + for _, o := range objects { + _, ok := s[Object(o)] + if !ok { + return false + } + } + return true +} + +// Insert adds all objects to the set +func (s Set[T]) Insert(objects ...T) Set[T] { + for _, o := range objects { + s[Object(o)] = o + } + return s +} + +// Delete removes all objects from the set +func (s Set[T]) Delete(objects ...T) Set[T] { + for _, o := range objects { + delete(s, Object(o)) + } + return s +} + +// Len returns the number of objects in the set +func (s Set[T]) Len() int { + return len(s) +} + +// Intersect returns a new set containing all the elements common to both. +// The new set will contain the object values from the receiver, not from the +// argument. +func (s Set[T]) Intersect(other Set[T]) Set[T] { + out := NewSet[T]() + for h, o := range s { + if _, ok := other[h]; ok { + out[h] = o + } + } + return out +} + +// SetDifference returns the set of objects in s that are not in other. +func (s Set[T]) SetDifference(other Set[T]) Set[T] { + result := NewSet[T]() + for h, o := range s { + if _, ok := other[h]; !ok { + result[h] = o + } + } + return result +} + +// Union returns the set of objects common to both sets. +func (s Set[T]) Union(other Set[T]) Set[T] { + result := NewSet[T]() + for h, o := range other { + result[h] = o + } + for h, o := range s { + result[h] = o + } + return result +} + +// Contains returns true if all elements of other can be found in s. +func (s Set[T]) Contains(other Set[T]) bool { + for h := range other { + if _, ok := s[h]; !ok { + return false + } + } + return true +} + +// EqualElements returns true if both sets have equal elements. +func (s Set[T]) EqualElements(other Set[T]) bool { + return s.Len() == other.Len() && s.Contains(other) +} + +// Pop removes and returns a single element from the set, and a bool indicating +// whether an element was found to pop. +func (s Set[T]) Pop() (T, bool) { + for h, o := range s { + delete(s, h) + return o, true + } + var zero T + return zero, false +} diff --git a/hash/set_test.go b/hash/set_test.go new file mode 100644 index 0000000..cf4740d --- /dev/null +++ b/hash/set_test.go @@ -0,0 +1,416 @@ +package hash + +import ( + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/authzed/controller-idioms/conditions" +) + +type MyObject struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + // this implements the conditions interface for MyObject, but note that + // this is not supported by kube codegen at the moment (don't try to use + // this in a real controller) + conditions.StatusWithConditions[*MyObjectStatus] `json:"-"` +} + +type MyObjectStatus struct { + ObservedGeneration int64 `json:"observedGeneration,omitempty" protobuf:"varint,3,opt,name=observedGeneration"` + conditions.StatusConditions `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"` +} + +// This test uses complex non-comparable objects to confirm the hashing behavior, +// but the rest of tests use string objects for simplicity +func TestNewSet(t *testing.T) { + testObject := MyObject{TypeMeta: metav1.TypeMeta{Kind: "test"}} + test2Object := MyObject{TypeMeta: metav1.TypeMeta{Kind: "test2"}} + objectWithConditions := MyObject{StatusWithConditions: conditions.StatusWithConditions[*MyObjectStatus]{Status: &MyObjectStatus{ + StatusConditions: conditions.StatusConditions{ + Conditions: []metav1.Condition{{Type: "Condition", Message: "happened"}}, + }, + }}} + objectWithTwoConditions := MyObject{StatusWithConditions: conditions.StatusWithConditions[*MyObjectStatus]{Status: &MyObjectStatus{ + StatusConditions: conditions.StatusConditions{ + Conditions: []metav1.Condition{ + {Type: "Condition", Message: "happened"}, + {Type: "Condition2", Message: "happened"}, + }, + }, + }}} + + type testCase[T any] struct { + name string + objects []T + want Set[T] + } + tests := []testCase[MyObject]{ + { + name: "with kube-like objects", + objects: []MyObject{ + testObject, + testObject, + test2Object, + }, + want: map[string]MyObject{ + Object(testObject): testObject, + Object(test2Object): test2Object, + }, + }, + { + name: "with equal non-comparable elements", + objects: []MyObject{ + objectWithConditions, + objectWithConditions, + }, + want: map[string]MyObject{ + Object(objectWithConditions): objectWithConditions, + }, + }, + { + name: "with non-equal non-comparable elements", + objects: []MyObject{ + objectWithConditions, + objectWithTwoConditions, + }, + want: map[string]MyObject{ + Object(objectWithConditions): objectWithConditions, + Object(objectWithTwoConditions): objectWithTwoConditions, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, NewSet(tt.objects...)) + }) + } +} + +func TestSet_Contains(t *testing.T) { + type testCase[T any] struct { + name string + s Set[T] + other Set[T] + want bool + } + tests := []testCase[string]{ + { + name: "contains", + s: NewSet("a", "b", "c"), + other: NewSet("b", "c"), + want: true, + }, + { + name: "does not contain", + s: NewSet("a", "b", "c"), + other: NewSet("b", "c", "d"), + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, tt.s.Contains(tt.other)) + }) + } +} + +func TestSet_Delete(t *testing.T) { + type testCase[T any] struct { + name string + s Set[T] + objects []T + want Set[T] + } + tests := []testCase[string]{ + { + name: "removes from set", + s: NewSet("a", "b", "c"), + objects: []string{"b", "c"}, + want: NewSet("a"), + }, + { + name: "no-op", + s: NewSet("a", "b", "c"), + objects: []string{"f"}, + want: NewSet("a", "b", "c"), + }, + { + name: "removes all", + s: NewSet("a", "b", "c"), + objects: []string{"a", "b", "c"}, + want: NewSet[string](), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, tt.s.Delete(tt.objects...)) + }) + } +} + +func TestSet_EqualElements(t *testing.T) { + type testCase[T any] struct { + name string + s Set[T] + other Set[T] + want bool + } + tests := []testCase[string]{ + { + name: "all elements equal", + s: NewSet("a", "b", "c"), + other: NewSet("a", "b", "c"), + want: true, + }, + { + name: "not equal", + s: NewSet("a", "b", "c"), + other: NewSet("a", "c"), + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, tt.s.EqualElements(tt.other)) + }) + } +} + +func TestSet_Has(t *testing.T) { + type testCase[T any] struct { + name string + s Set[T] + objects []T + want bool + } + tests := []testCase[string]{ + { + name: "has all objects", + s: NewSet("a", "b", "c"), + objects: []string{"b", "c"}, + want: true, + }, + { + name: "doesn't have all objects", + s: NewSet("a", "b", "c"), + objects: []string{"b", "d"}, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, tt.s.Has(tt.objects...)) + }) + } +} + +func TestSet_Insert(t *testing.T) { + type testCase[T any] struct { + name string + s Set[T] + objects []T + want Set[T] + } + tests := []testCase[string]{ + { + name: "insert new", + s: NewSet("a", "b", "c"), + objects: []string{"d", "e"}, + want: NewSet("a", "b", "c", "d", "e"), + }, + { + name: "insert existing", + s: NewSet("a", "b", "c"), + objects: []string{"a", "b"}, + want: NewSet("a", "b", "c"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, tt.s.Insert(tt.objects...)) + }) + } +} + +func TestSet_Intersect(t *testing.T) { + type testCase[T any] struct { + name string + s Set[T] + other Set[T] + want Set[T] + } + tests := []testCase[string]{ + { + name: "other subset of s", + s: NewSet("a", "b", "c"), + other: NewSet("a", "b"), + want: NewSet("a", "b"), + }, + { + name: "s subset of other", + s: NewSet("a", "b"), + other: NewSet("a", "b", "c"), + want: NewSet("a", "b"), + }, + { + name: "empty", + s: NewSet("a", "b"), + other: NewSet("c", "d"), + want: NewSet[string](), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, tt.s.Intersect(tt.other)) + }) + } +} + +func TestSet_Len(t *testing.T) { + type testCase[T any] struct { + name string + s Set[T] + want int + } + tests := []testCase[string]{ + { + name: "zero", + s: NewSet[string](), + want: 0, + }, + { + name: "nonzero", + s: NewSet("a", "b"), + want: 2, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, tt.s.Len()) + }) + } +} + +func TestSet_Pop(t *testing.T) { + type testCase[T any] struct { + name string + s Set[T] + want T + wantFound bool + } + tests := []testCase[string]{ + { + name: "returns last one", + s: NewSet("a"), + want: "a", + wantFound: true, + }, + { + name: "returns one from set", + s: NewSet("a", "b", "c"), + wantFound: true, + }, + { + name: "returns none when empty", + s: NewSet[string](), + want: "", + wantFound: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + initialLen := tt.s.Len() + got, found := tt.s.Pop() + require.Equal(t, tt.wantFound, found) + if initialLen < 2 { + require.Equal(t, tt.want, got) + } + if initialLen > 0 { + require.Equal(t, initialLen-1, tt.s.Len()) + } + require.False(t, tt.s.Has(got)) + }) + } +} + +func TestSet_SetDifference(t *testing.T) { + type testCase[T any] struct { + name string + s Set[T] + other Set[T] + want Set[T] + } + tests := []testCase[string]{ + { + name: "removes all", + s: NewSet("a", "b", "c"), + other: NewSet("a", "b", "c"), + want: NewSet[string](), + }, + { + name: "removes all from second set", + s: NewSet("a", "b", "c"), + other: NewSet("b", "c"), + want: NewSet("a"), + }, + { + name: "removes subset of second set", + s: NewSet("a", "b", "c"), + other: NewSet("b", "d"), + want: NewSet("a", "c"), + }, + { + name: "removes none", + s: NewSet("a", "b", "c"), + other: NewSet("e", "d"), + want: NewSet("a", "b", "c"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, tt.s.SetDifference(tt.other)) + }) + } +} + +func TestSet_Union(t *testing.T) { + type testCase[T any] struct { + name string + s Set[T] + other Set[T] + want Set[T] + } + tests := []testCase[string]{ + { + name: "disjoint", + s: NewSet("a", "b"), + other: NewSet("c", "d"), + want: NewSet("a", "b", "c", "d"), + }, + { + name: "s subset other", + s: NewSet("a", "b"), + other: NewSet("a", "b", "c"), + want: NewSet("a", "b", "c"), + }, + { + name: "other subset s", + s: NewSet("a", "b", "c"), + other: NewSet("a", "b"), + want: NewSet("a", "b", "c"), + }, + { + name: "equal", + s: NewSet("a", "b"), + other: NewSet("a", "b"), + want: NewSet("a", "b"), + }, + } + for _, tt := range tests { + require.Equal(t, tt.want, tt.s.Union(tt.other)) + } +} diff --git a/pause/pause_test.go b/pause/pause_test.go index 3220c69..69a93f3 100644 --- a/pause/pause_test.go +++ b/pause/pause_test.go @@ -25,6 +25,7 @@ type MyObject struct { // this in a real controller) conditions.StatusWithConditions[*MyObjectStatus] `json:"-"` } + type MyObjectStatus struct { ObservedGeneration int64 `json:"observedGeneration,omitempty" protobuf:"varint,3,opt,name=observedGeneration"` conditions.StatusConditions `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"`