Skip to content

Commit f89e037

Browse files
joelanfordjoelanford
committed
promote rukpak hash util to shared package, use in boxcutter applier
Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
1 parent 01cf0d3 commit f89e037

8 files changed

Lines changed: 54 additions & 85 deletions

File tree

‎internal/operator-controller/applier/boxcutter.go‎

Lines changed: 2 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,12 @@ package applier
33
import (
44
"cmp"
55
"context"
6-
"crypto/sha256"
7-
"encoding/hex"
86
"errors"
97
"fmt"
10-
"hash"
118
"io/fs"
129
"maps"
1310
"slices"
1411

15-
"github.com/davecgh/go-spew/spew"
1612
"k8s.io/apimachinery/pkg/api/meta"
1713
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1814
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -25,6 +21,7 @@ import (
2521
"github.com/operator-framework/operator-controller/internal/operator-controller/controllers"
2622
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source"
2723
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render"
24+
hashutil "github.com/operator-framework/operator-controller/internal/shared/util/hash"
2825
)
2926

3027
const (
@@ -127,7 +124,7 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
127124
if err != nil {
128125
return false, "", err
129126
}
130-
desiredHash := computeSHA256Hash(desiredRevision.Spec.Phases)
127+
desiredHash := hashutil.DeepHashObject(desiredRevision.Spec.Phases)
131128

132129
// Sort into current and previous revisions.
133130
var (
@@ -230,33 +227,6 @@ func (bc *Boxcutter) getExistingRevisions(ctx context.Context, extName string) (
230227
return existingRevisionList.Items, nil
231228
}
232229

233-
// computeSHA256Hash returns a sha236 hash value calculated from object.
234-
func computeSHA256Hash(obj any) string {
235-
hasher := sha256.New()
236-
deepHashObject(hasher, obj)
237-
return hex.EncodeToString(hasher.Sum(nil))
238-
}
239-
240-
// deepHashObject writes specified object to hash using the spew library
241-
// which follows pointers and prints actual values of the nested objects
242-
// ensuring the hash does not change when a pointer changes.
243-
func deepHashObject(hasher hash.Hash, objectToWrite any) {
244-
hasher.Reset()
245-
246-
// TODO: change this out to `json.Marshal`. Pretty sure we found issues in the past where
247-
// spew would produce different output when internal structures changed without the
248-
// external public API changing.
249-
printer := spew.ConfigState{
250-
Indent: " ",
251-
SortKeys: true,
252-
DisableMethods: true,
253-
SpewKeys: true,
254-
}
255-
if _, err := printer.Fprintf(hasher, "%#v", objectToWrite); err != nil {
256-
panic(err)
257-
}
258-
}
259-
260230
func latestRevisionNumber(prevRevisions []ocv1.ClusterExtensionRevision) int64 {
261231
if len(prevRevisions) == 0 {
262232
return 0

‎internal/operator-controller/applier/boxcutter_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ func TestBoxcutter_Apply(t *testing.T) {
268268
UID: "test-uid",
269269
},
270270
}
271-
defaultDesiredHash := "faaeb52a1cb7c968c96278bc1cd804e50d3ae9faae08807c9279a5e569933ea0"
271+
defaultDesiredHash := "gvvp8nzq5sbila80hkiv69am8hdr7o68qkk8n084gdn"
272272
defaultDesiredRevision := &ocv1.ClusterExtensionRevision{
273273
ObjectMeta: metav1.ObjectMeta{
274274
Name: "test-ext-1",
@@ -460,7 +460,7 @@ func TestBoxcutter_Apply(t *testing.T) {
460460

461461
assert.Equal(t, "test-ext-2", newRev.Name)
462462
assert.Equal(t, int64(2), newRev.Spec.Revision)
463-
assert.Equal(t, "ec8213d4061a75b55cd67a009d9cdeb1bdd6f503d4b3bb7b6cfea3a5233aad43", newRev.Annotations[applier.RevisionHashAnnotation])
463+
assert.Equal(t, "1fqrim12vefkogp3pwxwhcs7c0pi1z1t2fw4roxu81sv", newRev.Annotations[applier.RevisionHashAnnotation])
464464
require.Len(t, newRev.Spec.Previous, 1)
465465
assert.Equal(t, "test-ext-1", newRev.Spec.Previous[0].Name)
466466
assert.Equal(t, types.UID("rev-uid-1"), newRev.Spec.Previous[0].UID)

‎internal/operator-controller/rukpak/render/registryv1/generators/generators.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,7 @@ func BundleCSVPermissionsGenerator(rv1 *bundle.RegistryV1, opts render.Options)
122122
for _, ns := range opts.TargetNamespaces {
123123
for _, permission := range permissions {
124124
saName := saNameOrDefault(permission.ServiceAccountName)
125-
name, err := opts.UniqueNameGenerator(fmt.Sprintf("%s-%s", rv1.CSV.Name, saName), permission)
126-
if err != nil {
127-
return nil, err
128-
}
125+
name := opts.UniqueNameGenerator(fmt.Sprintf("%s-%s", rv1.CSV.Name, saName), permission)
129126

130127
objs = append(objs,
131128
CreateRoleResource(name, ns, WithRules(permission.Rules...)),
@@ -167,10 +164,7 @@ func BundleCSVClusterPermissionsGenerator(rv1 *bundle.RegistryV1, opts render.Op
167164
objs := make([]client.Object, 0, 2*len(clusterPermissions))
168165
for _, permission := range clusterPermissions {
169166
saName := saNameOrDefault(permission.ServiceAccountName)
170-
name, err := opts.UniqueNameGenerator(fmt.Sprintf("%s-%s", rv1.CSV.Name, saName), permission)
171-
if err != nil {
172-
return nil, err
173-
}
167+
name := opts.UniqueNameGenerator(fmt.Sprintf("%s-%s", rv1.CSV.Name, saName), permission)
174168
objs = append(objs,
175169
CreateClusterRoleResource(name, WithRules(permission.Rules...)),
176170
CreateClusterRoleBindingResource(

‎internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -352,8 +352,8 @@ func Test_BundleCSVDeploymentGenerator_FailsOnNil(t *testing.T) {
352352
}
353353

354354
func Test_BundleCSVPermissionsGenerator_Succeeds(t *testing.T) {
355-
fakeUniqueNameGenerator := func(base string, _ interface{}) (string, error) {
356-
return base, nil
355+
fakeUniqueNameGenerator := func(base string, _ interface{}) string {
356+
return base
357357
}
358358

359359
for _, tc := range []struct {
@@ -786,8 +786,8 @@ func Test_BundleCSVPermissionGenerator_FailsOnNil(t *testing.T) {
786786
}
787787

788788
func Test_BundleCSVClusterPermissionsGenerator_Succeeds(t *testing.T) {
789-
fakeUniqueNameGenerator := func(base string, _ interface{}) (string, error) {
790-
return base, nil
789+
fakeUniqueNameGenerator := func(base string, _ interface{}) string {
790+
return base
791791
}
792792

793793
for _, tc := range []struct {

‎internal/operator-controller/rukpak/render/render.go‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle"
1414
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util"
15+
hashutil "github.com/operator-framework/operator-controller/internal/shared/util/hash"
1516
)
1617

1718
// BundleValidator validates a RegistryV1 bundle by executing a series of
@@ -54,7 +55,7 @@ func (r ResourceGenerators) ResourceGenerator() ResourceGenerator {
5455
return r.GenerateResources
5556
}
5657

57-
type UniqueNameGenerator func(string, interface{}) (string, error)
58+
type UniqueNameGenerator func(string, interface{}) string
5859

5960
type Options struct {
6061
InstallNamespace string
@@ -138,12 +139,9 @@ func (r BundleRenderer) Render(rv1 bundle.RegistryV1, installNamespace string, o
138139
return objs, nil
139140
}
140141

141-
func DefaultUniqueNameGenerator(base string, o interface{}) (string, error) {
142-
hashStr, err := util.DeepHashObject(o)
143-
if err != nil {
144-
return "", err
145-
}
146-
return util.ObjectNameForBaseAndSuffix(base, hashStr), nil
142+
func DefaultUniqueNameGenerator(base string, o interface{}) string {
143+
hashStr := hashutil.DeepHashObject(o)
144+
return util.ObjectNameForBaseAndSuffix(base, hashStr)
147145
}
148146

149147
func validateTargetNamespaces(rv1 *bundle.RegistryV1, installNamespace string, targetNamespaces []string) error {

‎internal/operator-controller/rukpak/render/render_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -199,10 +199,10 @@ func Test_WithUniqueNameGenerator(t *testing.T) {
199199
opts := &render.Options{
200200
UniqueNameGenerator: render.DefaultUniqueNameGenerator,
201201
}
202-
render.WithUniqueNameGenerator(func(s string, i interface{}) (string, error) {
203-
return "a man needs a name", nil
202+
render.WithUniqueNameGenerator(func(s string, i interface{}) string {
203+
return "a man needs a name"
204204
})(opts)
205-
generatedName, _ := opts.UniqueNameGenerator("", nil)
205+
generatedName := opts.UniqueNameGenerator("", nil)
206206
require.Equal(t, "a man needs a name", generatedName)
207207
}
208208

internal/operator-controller/rukpak/util/hash.go renamed to internal/shared/util/hash/hash.go

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package util
1+
package hash
22

33
import (
44
"crypto/sha256"
@@ -10,7 +10,7 @@ import (
1010
// DeepHashObject writes specified object to hash using the spew library
1111
// which follows pointers and prints actual values of the nested objects
1212
// ensuring the hash does not change when a pointer changes.
13-
func DeepHashObject(obj interface{}) (string, error) {
13+
func DeepHashObject(obj interface{}) string {
1414
// While the most accurate encoding we could do for Kubernetes objects (runtime.Object)
1515
// would use the API machinery serializers, those operate over entire objects - and
1616
// we often need to operate on snippets. Checking with the experts and the implementation,
@@ -22,18 +22,17 @@ func DeepHashObject(obj interface{}) (string, error) {
2222
// will be encoded
2323

2424
hasher := sha256.New224()
25-
hasher.Reset()
2625
encoder := json.NewEncoder(hasher)
2726
if err := encoder.Encode(obj); err != nil {
28-
return "", fmt.Errorf("couldn't encode object: %w", err)
27+
panic(fmt.Sprintf("couldn't encode object: %v", err))
2928
}
3029

31-
// base62(sha224(bytes)) is a useful hash and encoding for adding the contents of this
30+
// base36(sha224(bytes)) is a useful hash and encoding for adding the contents of this
3231
// to a Kubernetes identifier or other field which has length and character set requirements
33-
var hash []byte
34-
hash = hasher.Sum(hash)
35-
36-
var i big.Int
37-
i.SetBytes(hash[:])
38-
return i.Text(36), nil
32+
var (
33+
i big.Int
34+
hash = make([]byte, 0, hasher.Size())
35+
)
36+
i.SetBytes(hasher.Sum(hash))
37+
return i.Text(36)
3938
}

internal/operator-controller/rukpak/util/hash_test.go renamed to internal/shared/util/hash/hash_test.go

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,30 @@
1-
package util_test
1+
package hash_test
22

33
import (
4+
"errors"
45
"testing"
56

67
"github.com/stretchr/testify/assert"
78
"github.com/stretchr/testify/require"
89

9-
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util"
10+
hashutil "github.com/operator-framework/operator-controller/internal/shared/util/hash"
1011
)
1112

13+
type unmarshalable struct{}
14+
15+
func (u *unmarshalable) MarshalJSON() ([]byte, error) {
16+
return nil, errors.New("unmarshalable")
17+
}
18+
1219
func TestDeepHashObject(t *testing.T) {
1320
tests := []struct {
1421
name string
15-
wantErr bool
22+
wantPanic bool
1623
obj interface{}
1724
expectedHash string
1825
}{
1926
{
20-
name: "populated obj with exported fields",
21-
wantErr: false,
27+
name: "populated obj with exported fields",
2228
obj: struct {
2329
Str string
2430
Num int
@@ -37,8 +43,7 @@ func TestDeepHashObject(t *testing.T) {
3743
expectedHash: "gta1bt5sybll5qjqxdiekmjm7py93glrinmnrfb31fj",
3844
},
3945
{
40-
name: "modified populated obj with exported fields",
41-
wantErr: false,
46+
name: "modified populated obj with exported fields",
4247
obj: struct {
4348
Str string
4449
Num int
@@ -57,8 +62,7 @@ func TestDeepHashObject(t *testing.T) {
5762
expectedHash: "1ftn1z2ieih8hsmi2a8c6mkoef6uodrtn4wtt1qapioh",
5863
},
5964
{
60-
name: "populated obj with unexported fields",
61-
wantErr: false,
65+
name: "populated obj with unexported fields",
6266
obj: struct {
6367
str string
6468
num int
@@ -80,38 +84,42 @@ func TestDeepHashObject(t *testing.T) {
8084
// The JSON encoder requires exported fields or it will generate
8185
// the same hash as a completely empty object
8286
name: "empty obj",
83-
wantErr: false,
8487
obj: struct{}{},
8588
expectedHash: "16jfjhihxbzhfhs1k5mimq740kvioi98pfbea9q6qtf9",
8689
},
8790
{
8891
name: "string a",
89-
wantErr: false,
9092
obj: "a",
9193
expectedHash: "1lu1qv1451mq7gv9upu1cx8ffffi07rel5xvbvvc44dh",
9294
},
9395
{
9496
name: "string b",
95-
wantErr: false,
9697
obj: "b",
9798
expectedHash: "1ija85ah4gd0beltpfhszipkxfyqqxhp94tf2mjfgq61",
9899
},
99100
{
100101
name: "nil obj",
101-
wantErr: false,
102102
obj: nil,
103103
expectedHash: "2im0kl1kwvzn46sr4cdtkvmdzrlurvj51xdzhwdht8l0",
104104
},
105+
{
106+
name: "unmarshalable obj",
107+
obj: &unmarshalable{},
108+
wantPanic: true,
109+
},
105110
}
106111
for _, tc := range tests {
107112
t.Run(tc.name, func(t *testing.T) {
108-
hash, err := util.DeepHashObject(tc.obj)
109-
if tc.wantErr {
110-
require.Error(t, err)
111-
} else {
112-
require.NoError(t, err)
113+
test := func() {
114+
hash := hashutil.DeepHashObject(tc.obj)
113115
assert.Equal(t, tc.expectedHash, hash)
114116
}
117+
118+
if tc.wantPanic {
119+
require.Panics(t, test)
120+
} else {
121+
require.NotPanics(t, test)
122+
}
115123
})
116124
}
117125
}

0 commit comments

Comments
 (0)