Skip to content

Commit

Permalink
Add CompareResources support for custom equals functions.
Browse files Browse the repository at this point in the history
The `cmp` functions are not performant. The reconciler will now support an
IsEqual function if it is defined, and will use it if a resource has one.
Additionally, one has been added for Okta assignments and dependent objects.
  • Loading branch information
mdwn authored and Mike Wilson committed Mar 7, 2024
1 parent 87ea6d4 commit 8d2d5f8
Show file tree
Hide file tree
Showing 13 changed files with 790 additions and 8 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/lint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,7 @@ jobs:
- name: Check if Operator CRDs are up to date
# We have to add the current directory as a safe directory or else git commands will not work as expected.
run: git config --global --add safe.directory $(realpath .) && make crds-up-to-date

- name: Check if derived functions are up to date
# We have to add the current directory as a safe directory or else git commands will not work as expected.
run: git config --global --add safe.directory $(realpath .) && make derive-up-to-date
13 changes: 13 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -1303,6 +1303,19 @@ buf/installed:
exit 1; \
fi

# derive will generate derived functions for our API.
.PHONY: derive
derive:
cd $(TOOLINGDIR) && go run ./cmd/goderive/main.go ../../api/types

# derive-up-to-date checks if the generated derived functions are up to date.
.PHONY: derive-up-to-date
derive-up-to-date: must-start-clean/host derive
@if ! $(GIT) diff --quiet; then \
echo 'Please run make derive.'; \
exit 1; \
fi

# grpc generates gRPC stubs from service definitions.
# This target runs in the buildbox container.
.PHONY: grpc
Expand Down
88 changes: 88 additions & 0 deletions api/types/derived.gen.go

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

8 changes: 8 additions & 0 deletions api/types/okta.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,14 @@ func (o *OktaAssignmentV1) CheckAndSetDefaults() error {
return nil
}

// IsEqual determines if two okta assignment resources are equivalent to one another.
func (o *OktaAssignmentV1) IsEqual(i OktaAssignment) bool {
if other, ok := i.(*OktaAssignmentV1); ok {
return deriveTeleportEqualOktaAssignmentV1(o, other)
}
return false
}

// OktaAssignmentTarget is an target for an Okta assignment.
type OktaAssignmentTarget interface {
// GetTargetType returns the target type.
Expand Down
176 changes: 176 additions & 0 deletions api/types/okta_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package types
import (
"fmt"
"testing"
"time"

"github.com/gravitational/trace"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -79,6 +80,181 @@ func TestOktaAssignments_SetStatus(t *testing.T) {
}
}

func TestOktAssignmentIsEqual(t *testing.T) {
newAssignment := func(changeFns ...func(*OktaAssignmentV1)) *OktaAssignmentV1 {
assignment := &OktaAssignmentV1{
ResourceHeader: ResourceHeader{
Kind: KindOktaAssignment,
Version: V1,
Metadata: Metadata{
Name: "name",
},
},
Spec: OktaAssignmentSpecV1{
User: "user",
Targets: []*OktaAssignmentTargetV1{
{Id: "1", Type: OktaAssignmentTargetV1_APPLICATION},
{Id: "2", Type: OktaAssignmentTargetV1_GROUP},
},
CleanupTime: time.Time{},
Status: OktaAssignmentSpecV1_PENDING,
LastTransition: time.Time{},
Finalized: true,
},
}
require.NoError(t, assignment.CheckAndSetDefaults())

for _, fn := range changeFns {
fn(assignment)
}

return assignment
}
tests := []struct {
name string
o1 *OktaAssignmentV1
o2 *OktaAssignmentV1
expected bool
}{
{
name: "empty equals",
o1: &OktaAssignmentV1{},
o2: &OktaAssignmentV1{},
expected: true,
},
{
name: "nil equals",
o1: nil,
o2: (*OktaAssignmentV1)(nil),
expected: true,
},
{
name: "one is nil",
o1: &OktaAssignmentV1{},
o2: (*OktaAssignmentV1)(nil),
expected: false,
},
{
name: "populated equals",
o1: newAssignment(),
o2: newAssignment(),
expected: true,
},
{
name: "resource header is different",
o1: newAssignment(),
o2: newAssignment(func(o *OktaAssignmentV1) {
o.ResourceHeader.Kind = "different-kind"
}),
expected: false,
},
{
name: "user is different",
o1: newAssignment(),
o2: newAssignment(func(o *OktaAssignmentV1) {
o.Spec.User = "different-user"
}),
expected: false,
},
{
name: "targets different id",
o1: newAssignment(),
o2: newAssignment(func(o *OktaAssignmentV1) {
o.Spec.Targets = []*OktaAssignmentTargetV1{
{Id: "2", Type: OktaAssignmentTargetV1_APPLICATION},
{Id: "2", Type: OktaAssignmentTargetV1_GROUP},
}
}),
expected: false,
},
{
name: "targets different type",
o1: newAssignment(),
o2: newAssignment(func(o *OktaAssignmentV1) {
o.Spec.Targets = []*OktaAssignmentTargetV1{
{Id: "1", Type: OktaAssignmentTargetV1_GROUP},
{Id: "2", Type: OktaAssignmentTargetV1_GROUP},
}
}),
expected: false,
},
{
name: "targets different sizes",
o1: newAssignment(),
o2: newAssignment(func(o *OktaAssignmentV1) {
o.Spec.Targets = []*OktaAssignmentTargetV1{
{Id: "1", Type: OktaAssignmentTargetV1_APPLICATION},
}
}),
expected: false,
},
{
name: "targets both nil",
o1: newAssignment(func(o *OktaAssignmentV1) {
o.Spec.Targets = nil
}),
o2: newAssignment(func(o *OktaAssignmentV1) {
o.Spec.Targets = nil
}),
expected: true,
},
{
name: "targets o1 is nil",
o1: newAssignment(func(o *OktaAssignmentV1) {
o.Spec.Targets = nil
}),
o2: newAssignment(),
expected: false,
},
{
name: "targets o2 is nil",
o1: newAssignment(),
o2: newAssignment(func(o *OktaAssignmentV1) {
o.Spec.Targets = nil
}),
expected: false,
},
{
name: "cleanup time is different",
o1: newAssignment(),
o2: newAssignment(func(o *OktaAssignmentV1) {
o.Spec.CleanupTime = time.Date(1, 2, 3, 4, 5, 6, 7, time.UTC)
}),
expected: false,
},
{
name: "status is different",
o1: newAssignment(),
o2: newAssignment(func(o *OktaAssignmentV1) {
o.Spec.Status = OktaAssignmentSpecV1_PROCESSING
}),
expected: false,
},
{
name: "last transition is different",
o1: newAssignment(),
o2: newAssignment(func(o *OktaAssignmentV1) {
o.Spec.CleanupTime = time.Date(1, 2, 3, 4, 5, 6, 7, time.UTC)
}),
expected: false,
},
{
name: "finalized is different",
o1: newAssignment(),
o2: newAssignment(func(o *OktaAssignmentV1) {
o.Spec.Finalized = false
}),
expected: false,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
require.Equal(t, test.expected, test.o1.IsEqual(test.o2))
})
}
}

func newOktaAssignment(t *testing.T, status string) OktaAssignment {
assignment := &OktaAssignmentV1{}

Expand Down
10 changes: 10 additions & 0 deletions api/types/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,11 @@ func (h *ResourceHeader) GetAllLabels() map[string]string {
return h.Metadata.Labels
}

// IsEqual determines if two resource header resources are equivalent to one another.
func (h *ResourceHeader) IsEqual(other *ResourceHeader) bool {
return deriveTeleportEqualResourceHeader(h, other)
}

func (h *ResourceHeader) CheckAndSetDefaults() error {
if h.Kind == "" {
return trace.BadParameter("resource has an empty Kind field")
Expand Down Expand Up @@ -452,6 +457,11 @@ func (m *Metadata) SetOrigin(origin string) {
m.Labels[OriginLabel] = origin
}

// IsEqual determines if two metadata resources are equivalent to one another.
func (m *Metadata) IsEqual(other *Metadata) bool {
return deriveTeleportEqualMetadata(m, other)
}

// CheckAndSetDefaults checks validity of all parameters and sets defaults
func (m *Metadata) CheckAndSetDefaults() error {
if m.Name == "" {
Expand Down
Loading

0 comments on commit 8d2d5f8

Please sign in to comment.