From 4ef1cf2f3bc3c3e2df949a93ab8ebc87882861c7 Mon Sep 17 00:00:00 2001 From: Hugo Shaka Date: Tue, 5 Nov 2024 09:59:32 -0500 Subject: [PATCH] [v17] enforce conditional updates on AutoUpdate* + rename typos (#48390) * enforce conditaional updates on AutoUpdate* + rename typos * fix tests --- api/types/autoupdate/rollout.go | 6 +++--- lib/services/autoupdates.go | 6 +++--- lib/services/local/autoupdate.go | 6 +++--- lib/services/local/autoupdate_test.go | 10 ++++++++-- tool/tctl/common/edit_command_test.go | 4 ++-- 5 files changed, 19 insertions(+), 13 deletions(-) diff --git a/api/types/autoupdate/rollout.go b/api/types/autoupdate/rollout.go index 814d71313d3ed..d935244af31b3 100644 --- a/api/types/autoupdate/rollout.go +++ b/api/types/autoupdate/rollout.go @@ -26,7 +26,7 @@ import ( // NewAutoUpdateAgentRollout creates a new auto update version resource. func NewAutoUpdateAgentRollout(spec *autoupdate.AutoUpdateAgentRolloutSpec) (*autoupdate.AutoUpdateAgentRollout, error) { - version := &autoupdate.AutoUpdateAgentRollout{ + rollout := &autoupdate.AutoUpdateAgentRollout{ Kind: types.KindAutoUpdateAgentRollout, Version: types.V1, Metadata: &headerv1.Metadata{ @@ -34,11 +34,11 @@ func NewAutoUpdateAgentRollout(spec *autoupdate.AutoUpdateAgentRolloutSpec) (*au }, Spec: spec, } - if err := ValidateAutoUpdateAgentRollout(version); err != nil { + if err := ValidateAutoUpdateAgentRollout(rollout); err != nil { return nil, trace.Wrap(err) } - return version, nil + return rollout, nil } // ValidateAutoUpdateAgentRollout checks that required parameters are set diff --git a/lib/services/autoupdates.go b/lib/services/autoupdates.go index 72d51b4ac2338..f57d384df2dde 100644 --- a/lib/services/autoupdates.go +++ b/lib/services/autoupdates.go @@ -65,13 +65,13 @@ type AutoUpdateService interface { DeleteAutoUpdateVersion(ctx context.Context) error // CreateAutoUpdateAgentRollout creates the AutoUpdateAgentRollout singleton resource. - CreateAutoUpdateAgentRollout(ctx context.Context, plan *autoupdate.AutoUpdateAgentRollout) (*autoupdate.AutoUpdateAgentRollout, error) + CreateAutoUpdateAgentRollout(ctx context.Context, rollout *autoupdate.AutoUpdateAgentRollout) (*autoupdate.AutoUpdateAgentRollout, error) // UpdateAutoUpdateAgentRollout updates the AutoUpdateAgentRollout singleton resource. - UpdateAutoUpdateAgentRollout(ctx context.Context, plan *autoupdate.AutoUpdateAgentRollout) (*autoupdate.AutoUpdateAgentRollout, error) + UpdateAutoUpdateAgentRollout(ctx context.Context, rollout *autoupdate.AutoUpdateAgentRollout) (*autoupdate.AutoUpdateAgentRollout, error) // UpsertAutoUpdateAgentRollout sets the AutoUpdateAgentRollout singleton resource. - UpsertAutoUpdateAgentRollout(ctx context.Context, plan *autoupdate.AutoUpdateAgentRollout) (*autoupdate.AutoUpdateAgentRollout, error) + UpsertAutoUpdateAgentRollout(ctx context.Context, rollout *autoupdate.AutoUpdateAgentRollout) (*autoupdate.AutoUpdateAgentRollout, error) // DeleteAutoUpdateAgentRollout deletes the AutoUpdateAgentRollout singleton resource. DeleteAutoUpdateAgentRollout(ctx context.Context) error diff --git a/lib/services/local/autoupdate.go b/lib/services/local/autoupdate.go index 879e5348d1d4e..93a5142ca81a1 100644 --- a/lib/services/local/autoupdate.go +++ b/lib/services/local/autoupdate.go @@ -113,7 +113,7 @@ func (s *AutoUpdateService) UpdateAutoUpdateConfig( ctx context.Context, c *autoupdate.AutoUpdateConfig, ) (*autoupdate.AutoUpdateConfig, error) { - config, err := s.config.UpdateResource(ctx, c) + config, err := s.config.ConditionalUpdateResource(ctx, c) return config, trace.Wrap(err) } @@ -151,7 +151,7 @@ func (s *AutoUpdateService) UpdateAutoUpdateVersion( ctx context.Context, v *autoupdate.AutoUpdateVersion, ) (*autoupdate.AutoUpdateVersion, error) { - version, err := s.version.UpdateResource(ctx, v) + version, err := s.version.ConditionalUpdateResource(ctx, v) return version, trace.Wrap(err) } @@ -189,7 +189,7 @@ func (s *AutoUpdateService) UpdateAutoUpdateAgentRollout( ctx context.Context, v *autoupdate.AutoUpdateAgentRollout, ) (*autoupdate.AutoUpdateAgentRollout, error) { - rollout, err := s.rollout.UpdateResource(ctx, v) + rollout, err := s.rollout.ConditionalUpdateResource(ctx, v) return rollout, trace.Wrap(err) } diff --git a/lib/services/local/autoupdate_test.go b/lib/services/local/autoupdate_test.go index ae858d65cc47c..a992322472975 100644 --- a/lib/services/local/autoupdate_test.go +++ b/lib/services/local/autoupdate_test.go @@ -93,8 +93,11 @@ func TestAutoUpdateServiceConfigCRUD(t *testing.T) { var notFoundError *trace.NotFoundError require.ErrorAs(t, err, ¬FoundError) + // If we try to conditionally update a missing resource, we receive + // a CompareFailed instead of a NotFound. + var revisionMismatchError *trace.CompareFailedError _, err = service.UpdateAutoUpdateConfig(ctx, config) - require.ErrorAs(t, err, ¬FoundError) + require.ErrorAs(t, err, &revisionMismatchError) } // TestAutoUpdateServiceVersionCRUD verifies get/create/update/upsert/delete methods of the backend service @@ -155,8 +158,11 @@ func TestAutoUpdateServiceVersionCRUD(t *testing.T) { var notFoundError *trace.NotFoundError require.ErrorAs(t, err, ¬FoundError) + // If we try to conditionally update a missing resource, we receive + // a CompareFailed instead of a NotFound. + var revisionMismatchError *trace.CompareFailedError _, err = service.UpdateAutoUpdateVersion(ctx, version) - require.ErrorAs(t, err, ¬FoundError) + require.ErrorAs(t, err, &revisionMismatchError) } // TestAutoUpdateServiceInvalidNameCreate verifies that configuration and version diff --git a/tool/tctl/common/edit_command_test.go b/tool/tctl/common/edit_command_test.go index c0ffbf342a485..7d3ddc98eabab 100644 --- a/tool/tctl/common/edit_command_test.go +++ b/tool/tctl/common/edit_command_test.go @@ -574,7 +574,7 @@ func testEditAutoUpdateConfig(t *testing.T, clt *authclient.Client) { require.NoError(t, err) serviceClient := autoupdatev1pb.NewAutoUpdateServiceClient(clt.GetConnection()) - _, err = serviceClient.CreateAutoUpdateConfig(ctx, &autoupdatev1pb.CreateAutoUpdateConfigRequest{Config: initial}) + initial, err = serviceClient.CreateAutoUpdateConfig(ctx, &autoupdatev1pb.CreateAutoUpdateConfigRequest{Config: initial}) require.NoError(t, err, "creating initial autoupdate config") editor := func(name string) error { @@ -616,7 +616,7 @@ func testEditAutoUpdateVersion(t *testing.T, clt *authclient.Client) { require.NoError(t, err) serviceClient := autoupdatev1pb.NewAutoUpdateServiceClient(clt.GetConnection()) - _, err = serviceClient.CreateAutoUpdateVersion(ctx, &autoupdatev1pb.CreateAutoUpdateVersionRequest{Version: initial}) + initial, err = serviceClient.CreateAutoUpdateVersion(ctx, &autoupdatev1pb.CreateAutoUpdateVersionRequest{Version: initial}) require.NoError(t, err, "creating initial autoupdate version") editor := func(name string) error {