Skip to content

Commit f8b41d6

Browse files
committed
fix: correctly compare resources when specs are embedded proto structs
When resource spec is a protobuf-generated struct, resource.Equal was working incorrectly as proto types can't be compared correctly with reflect.DeepEqual. Signed-off-by: Andrey Smirnov <andrey.smirnov@talos-systems.com>
1 parent 1ed3207 commit f8b41d6

3 files changed

Lines changed: 73 additions & 1 deletion

File tree

pkg/resource/protobuf/spec.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,16 @@ func (spec ResourceSpec[T, S]) GetValue() proto.Message { //nolint:ireturn
4545
return spec.Value
4646
}
4747

48+
// Equal implements spec equality check.
49+
func (spec ResourceSpec[T, S]) Equal(other interface{}) bool {
50+
otherSpec, ok := other.(ResourceSpec[T, S])
51+
if !ok {
52+
return false
53+
}
54+
55+
return proto.Equal(spec.Value, otherSpec.Value)
56+
}
57+
4858
// NewResourceSpec creates new ResourceSpec[T, S].
4959
// T is a protobuf generated structure.
5060
// S is a pointer to T.

pkg/resource/protobuf/spec_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,17 @@ import (
1111
"google.golang.org/protobuf/proto"
1212

1313
"github.com/cosi-project/runtime/api/v1alpha1"
14+
"github.com/cosi-project/runtime/pkg/resource"
15+
"github.com/cosi-project/runtime/pkg/resource/meta"
1416
"github.com/cosi-project/runtime/pkg/resource/protobuf"
17+
"github.com/cosi-project/runtime/pkg/resource/typed"
1518
)
1619

1720
type testSpec = protobuf.ResourceSpec[v1alpha1.Metadata, *v1alpha1.Metadata]
1821

1922
func TestResourceSpec(t *testing.T) {
23+
t.Parallel()
24+
2025
spec := protobuf.NewResourceSpec(&v1alpha1.Metadata{
2126
Version: "v0.1.0",
2227
})
@@ -30,3 +35,52 @@ func TestResourceSpec(t *testing.T) {
3035

3136
require.True(t, proto.Equal(spec.Value, specDecoded.Value))
3237
}
38+
39+
type testResource = typed.Resource[testSpec, testRD]
40+
41+
type testRD struct{} //nolint:unused
42+
43+
//nolint:unused
44+
func (testRD) ResourceDefinition(md resource.Metadata, spec testSpec) meta.ResourceDefinitionSpec {
45+
return meta.ResourceDefinitionSpec{
46+
Type: "testResource",
47+
}
48+
}
49+
50+
func TestResourceEquality(t *testing.T) {
51+
t.Parallel()
52+
53+
r1 := typed.NewResource[testSpec, testRD](resource.NewMetadata("default", "testResource", "aaa", resource.VersionUndefined), protobuf.NewResourceSpec(&v1alpha1.Metadata{
54+
Version: "v0.1.0",
55+
}))
56+
57+
r2 := typed.NewResource[testSpec, testRD](resource.NewMetadata("default", "testResource", "aaa", resource.VersionUndefined), protobuf.NewResourceSpec(&v1alpha1.Metadata{
58+
Version: "v0.1.0",
59+
}))
60+
61+
// two manually created resources should be equal
62+
require.True(t, resource.Equal(r1, r2))
63+
64+
// pass r1 through marshal/unmarshal stage, and make sure it's still equal to itself
65+
protoR, err := protobuf.FromResource(r1)
66+
require.NoError(t, err)
67+
68+
pR, err := protoR.Marshal()
69+
require.NoError(t, err)
70+
71+
protoR, err = protobuf.Unmarshal(pR)
72+
require.NoError(t, err)
73+
74+
r3, err := protobuf.UnmarshalResource(protoR)
75+
require.NoError(t, err)
76+
77+
// unmarshaling fills in some private fields in protobuf Go struct which should be ignored
78+
// on equality check
79+
require.True(t, resource.Equal(r1, r3))
80+
}
81+
82+
func init() {
83+
if err := protobuf.RegisterResource("testResource", &testResource{}); err != nil {
84+
panic(err)
85+
}
86+
}

pkg/resource/resource.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,15 @@ func Equal(r1, r2 Resource) bool {
4545
return false
4646
}
4747

48-
return reflect.DeepEqual(r1.Spec(), r2.Spec())
48+
spec1, spec2 := r1.Spec(), r2.Spec()
49+
50+
if equality, ok := spec1.(interface {
51+
Equal(interface{}) bool
52+
}); ok {
53+
return equality.Equal(spec2)
54+
}
55+
56+
return reflect.DeepEqual(spec1, spec2)
4957
}
5058

5159
// MarshalYAML marshals resource to YAML definition.

0 commit comments

Comments
 (0)