From 0a9f4468935a577408f75d8a714ce449ce171471 Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Wed, 10 Feb 2021 20:59:46 -0500 Subject: [PATCH] Update credential issuer logic to use status subresource Signed-off-by: Monis Khan --- ...eate_or_update_credential_issuer_config.go | 45 ++++++------ ...or_update_credential_issuer_config_test.go | 68 +++++++++++-------- .../kube_config_info_publisher.go | 6 +- .../kube_config_info_publisher_test.go | 43 +++++++++--- .../kubecertagent/annotater_test.go | 16 ++++- .../controller/kubecertagent/creater_test.go | 40 ++++++++++- .../controller/kubecertagent/execer_test.go | 38 +++++++++-- .../controller/kubecertagent/kubecertagent.go | 6 +- test/integration/kubeclient_test.go | 3 - 9 files changed, 183 insertions(+), 82 deletions(-) diff --git a/internal/controller/issuerconfig/create_or_update_credential_issuer_config.go b/internal/controller/issuerconfig/create_or_update_credential_issuer_config.go index 859dc721..1aff2208 100644 --- a/internal/controller/issuerconfig/create_or_update_credential_issuer_config.go +++ b/internal/controller/issuerconfig/create_or_update_credential_issuer_config.go @@ -17,12 +17,12 @@ import ( pinnipedclientset "go.pinniped.dev/generated/1.20/client/concierge/clientset/versioned" ) -func CreateOrUpdateCredentialIssuer( +func CreateOrUpdateCredentialIssuerStatus( ctx context.Context, credentialIssuerResourceName string, credentialIssuerLabels map[string]string, pinnipedClient pinnipedclientset.Interface, - applyUpdatesToCredentialIssuerFunc func(configToUpdate *configv1alpha1.CredentialIssuer), + applyUpdatesToCredentialIssuerFunc func(configToUpdate *configv1alpha1.CredentialIssuerStatus), ) error { err := retry.RetryOnConflict(retry.DefaultRetry, func() error { existingCredentialIssuer, err := pinnipedClient. @@ -38,29 +38,30 @@ func CreateOrUpdateCredentialIssuer( credentialIssuersClient := pinnipedClient.ConfigV1alpha1().CredentialIssuers() if notFound { - // Create it - credentialIssuer := minimalValidCredentialIssuer( - credentialIssuerResourceName, credentialIssuerLabels, - ) - applyUpdatesToCredentialIssuerFunc(credentialIssuer) + // create an empty credential issuer + minCredentialIssuer := minimalValidCredentialIssuer(credentialIssuerResourceName, credentialIssuerLabels) - if _, err := credentialIssuersClient.Create(ctx, credentialIssuer, metav1.CreateOptions{}); err != nil { + newCredentialIssuer, err := credentialIssuersClient.Create(ctx, minCredentialIssuer, metav1.CreateOptions{}) + if err != nil { return fmt.Errorf("create failed: %w", err) } - } else { - // Already exists, so check to see if we need to update it - credentialIssuer := existingCredentialIssuer.DeepCopy() - applyUpdatesToCredentialIssuerFunc(credentialIssuer) - if equality.Semantic.DeepEqual(existingCredentialIssuer, credentialIssuer) { - // Nothing interesting would change as a result of this update, so skip it - return nil - } - - if _, err := credentialIssuersClient.Update(ctx, credentialIssuer, metav1.UpdateOptions{}); err != nil { - return err - } + existingCredentialIssuer = newCredentialIssuer } + + // check to see if we need to update the status + credentialIssuer := existingCredentialIssuer.DeepCopy() + applyUpdatesToCredentialIssuerFunc(&credentialIssuer.Status) + + if equality.Semantic.DeepEqual(existingCredentialIssuer, credentialIssuer) { + // Nothing interesting would change as a result of this update, so skip it + return nil + } + + if _, err := credentialIssuersClient.UpdateStatus(ctx, credentialIssuer, metav1.UpdateOptions{}); err != nil { + return err + } + return nil }) @@ -80,9 +81,5 @@ func minimalValidCredentialIssuer( Name: credentialIssuerName, Labels: credentialIssuerLabels, }, - Status: configv1alpha1.CredentialIssuerStatus{ - Strategies: []configv1alpha1.CredentialIssuerStrategy{}, - KubeConfigInfo: nil, - }, } } diff --git a/internal/controller/issuerconfig/create_or_update_credential_issuer_config_test.go b/internal/controller/issuerconfig/create_or_update_credential_issuer_config_test.go index 9745545e..1b1c865a 100644 --- a/internal/controller/issuerconfig/create_or_update_credential_issuer_config_test.go +++ b/internal/controller/issuerconfig/create_or_update_credential_issuer_config_test.go @@ -23,7 +23,7 @@ import ( pinnipedfake "go.pinniped.dev/generated/1.20/client/concierge/clientset/versioned/fake" ) -func TestCreateOrUpdateCredentialIssuer(t *testing.T) { +func TestCreateOrUpdateCredentialIssuerStatus(t *testing.T) { spec.Run(t, "specs", func(t *testing.T, when spec.G, it spec.S) { var r *require.Assertions var ctx context.Context @@ -43,8 +43,8 @@ func TestCreateOrUpdateCredentialIssuer(t *testing.T) { }) when("the config does not exist", func() { - it("creates a new config which includes only the updates made by the func parameter", func() { - err := CreateOrUpdateCredentialIssuer( + it("creates a new config and then updates it with the func parameter", func() { + err := CreateOrUpdateCredentialIssuerStatus( ctx, credentialIssuerResourceName, map[string]string{ @@ -52,8 +52,8 @@ func TestCreateOrUpdateCredentialIssuer(t *testing.T) { "myLabelKey2": "myLabelValue2", }, pinnipedAPIClient, - func(configToUpdate *configv1alpha1.CredentialIssuer) { - configToUpdate.Status.KubeConfigInfo = &configv1alpha1.CredentialIssuerKubeConfigInfo{ + func(configToUpdate *configv1alpha1.CredentialIssuerStatus) { + configToUpdate.KubeConfigInfo = &configv1alpha1.CredentialIssuerKubeConfigInfo{ CertificateAuthorityData: "some-ca-value", } }, @@ -64,6 +64,19 @@ func TestCreateOrUpdateCredentialIssuer(t *testing.T) { expectedCreateAction := coretesting.NewRootCreateAction( credentialIssuerGVR, + &configv1alpha1.CredentialIssuer{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: credentialIssuerResourceName, + Labels: map[string]string{ + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", + }, + }, + }, + ) + + expectedUpdateAction := coretesting.NewRootUpdateSubresourceAction(credentialIssuerGVR, "status", &configv1alpha1.CredentialIssuer{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ @@ -74,7 +87,6 @@ func TestCreateOrUpdateCredentialIssuer(t *testing.T) { }, }, Status: configv1alpha1.CredentialIssuerStatus{ - Strategies: []configv1alpha1.CredentialIssuerStrategy{}, KubeConfigInfo: &configv1alpha1.CredentialIssuerKubeConfigInfo{ Server: "", CertificateAuthorityData: "some-ca-value", @@ -83,7 +95,7 @@ func TestCreateOrUpdateCredentialIssuer(t *testing.T) { }, ) - r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction}, pinnipedAPIClient.Actions()) + r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction, expectedUpdateAction}, pinnipedAPIClient.Actions()) }) when("there is an unexpected error while creating the existing object", func() { @@ -94,12 +106,12 @@ func TestCreateOrUpdateCredentialIssuer(t *testing.T) { }) it("returns an error", func() { - err := CreateOrUpdateCredentialIssuer( + err := CreateOrUpdateCredentialIssuerStatus( ctx, credentialIssuerResourceName, map[string]string{}, pinnipedAPIClient, - func(configToUpdate *configv1alpha1.CredentialIssuer) {}, + func(configToUpdate *configv1alpha1.CredentialIssuerStatus) {}, ) r.EqualError(err, "could not create or update credentialissuer: create failed: error on create") }) @@ -138,7 +150,7 @@ func TestCreateOrUpdateCredentialIssuer(t *testing.T) { }) it("updates the existing config to only apply the updates made by the func parameter", func() { - err := CreateOrUpdateCredentialIssuer( + err := CreateOrUpdateCredentialIssuerStatus( ctx, credentialIssuerResourceName, map[string]string{ @@ -146,8 +158,8 @@ func TestCreateOrUpdateCredentialIssuer(t *testing.T) { "myLabelKey2": "myLabelValue2", }, pinnipedAPIClient, - func(configToUpdate *configv1alpha1.CredentialIssuer) { - configToUpdate.Status.KubeConfigInfo.CertificateAuthorityData = "new-ca-value" + func(configToUpdate *configv1alpha1.CredentialIssuerStatus) { + configToUpdate.KubeConfigInfo.CertificateAuthorityData = "new-ca-value" }, ) r.NoError(err) @@ -157,24 +169,24 @@ func TestCreateOrUpdateCredentialIssuer(t *testing.T) { // Only the edited field should be changed. expectedUpdatedConfig := existingConfig.DeepCopy() expectedUpdatedConfig.Status.KubeConfigInfo.CertificateAuthorityData = "new-ca-value" - expectedUpdateAction := coretesting.NewRootUpdateAction(credentialIssuerGVR, expectedUpdatedConfig) + expectedUpdateAction := coretesting.NewRootUpdateSubresourceAction(credentialIssuerGVR, "status", expectedUpdatedConfig) r.Equal([]coretesting.Action{expectedGetAction, expectedUpdateAction}, pinnipedAPIClient.Actions()) }) it("avoids the cost of an update if the local updates made by the func parameter did not actually change anything", func() { - err := CreateOrUpdateCredentialIssuer( + err := CreateOrUpdateCredentialIssuerStatus( ctx, credentialIssuerResourceName, map[string]string{}, pinnipedAPIClient, - func(configToUpdate *configv1alpha1.CredentialIssuer) { - configToUpdate.Status.KubeConfigInfo.CertificateAuthorityData = "initial-ca-value" + func(configToUpdate *configv1alpha1.CredentialIssuerStatus) { + configToUpdate.KubeConfigInfo.CertificateAuthorityData = "initial-ca-value" - t := configToUpdate.Status.Strategies[0].LastUpdateTime + t := configToUpdate.Strategies[0].LastUpdateTime loc, err := time.LoadLocation("Asia/Shanghai") r.NoError(err) - configToUpdate.Status.Strategies[0].LastUpdateTime = metav1.NewTime(t.In(loc)) + configToUpdate.Strategies[0].LastUpdateTime = metav1.NewTime(t.In(loc)) }, ) r.NoError(err) @@ -191,12 +203,12 @@ func TestCreateOrUpdateCredentialIssuer(t *testing.T) { }) it("returns an error", func() { - err := CreateOrUpdateCredentialIssuer( + err := CreateOrUpdateCredentialIssuerStatus( ctx, credentialIssuerResourceName, map[string]string{}, pinnipedAPIClient, - func(configToUpdate *configv1alpha1.CredentialIssuer) {}, + func(configToUpdate *configv1alpha1.CredentialIssuerStatus) {}, ) r.EqualError(err, "could not create or update credentialissuer: get failed: error on get") }) @@ -210,13 +222,13 @@ func TestCreateOrUpdateCredentialIssuer(t *testing.T) { }) it("returns an error", func() { - err := CreateOrUpdateCredentialIssuer( + err := CreateOrUpdateCredentialIssuerStatus( ctx, credentialIssuerResourceName, map[string]string{}, pinnipedAPIClient, - func(configToUpdate *configv1alpha1.CredentialIssuer) { - configToUpdate.Status.KubeConfigInfo.CertificateAuthorityData = "new-ca-value" + func(configToUpdate *configv1alpha1.CredentialIssuerStatus) { + configToUpdate.KubeConfigInfo.CertificateAuthorityData = "new-ca-value" }, ) r.EqualError(err, "could not create or update credentialissuer: error on update") @@ -248,7 +260,7 @@ func TestCreateOrUpdateCredentialIssuer(t *testing.T) { }) it("retries updates on conflict", func() { - err := CreateOrUpdateCredentialIssuer( + err := CreateOrUpdateCredentialIssuerStatus( ctx, credentialIssuerResourceName, map[string]string{ @@ -256,8 +268,8 @@ func TestCreateOrUpdateCredentialIssuer(t *testing.T) { "myLabelKey2": "myLabelValue2", }, pinnipedAPIClient, - func(configToUpdate *configv1alpha1.CredentialIssuer) { - configToUpdate.Status.KubeConfigInfo.CertificateAuthorityData = "new-ca-value" + func(configToUpdate *configv1alpha1.CredentialIssuerStatus) { + configToUpdate.KubeConfigInfo.CertificateAuthorityData = "new-ca-value" }, ) r.NoError(err) @@ -267,13 +279,13 @@ func TestCreateOrUpdateCredentialIssuer(t *testing.T) { // The first attempted update only includes its own edits. firstExpectedUpdatedConfig := existingConfig.DeepCopy() firstExpectedUpdatedConfig.Status.KubeConfigInfo.CertificateAuthorityData = "new-ca-value" - firstExpectedUpdateAction := coretesting.NewRootUpdateAction(credentialIssuerGVR, firstExpectedUpdatedConfig) + firstExpectedUpdateAction := coretesting.NewRootUpdateSubresourceAction(credentialIssuerGVR, "status", firstExpectedUpdatedConfig) // Both the edits made by this update and the edits made by the conflicting update should be included. secondExpectedUpdatedConfig := existingConfig.DeepCopy() secondExpectedUpdatedConfig.Status.KubeConfigInfo.Server = "some-other-server-value-from-conflicting-update" secondExpectedUpdatedConfig.Status.KubeConfigInfo.CertificateAuthorityData = "new-ca-value" - secondExpectedUpdateAction := coretesting.NewRootUpdateAction(credentialIssuerGVR, secondExpectedUpdatedConfig) + secondExpectedUpdateAction := coretesting.NewRootUpdateSubresourceAction(credentialIssuerGVR, "status", secondExpectedUpdatedConfig) expectedActions := []coretesting.Action{ expectedGetAction, diff --git a/internal/controller/issuerconfig/kube_config_info_publisher.go b/internal/controller/issuerconfig/kube_config_info_publisher.go index 09f883b4..494a6376 100644 --- a/internal/controller/issuerconfig/kube_config_info_publisher.go +++ b/internal/controller/issuerconfig/kube_config_info_publisher.go @@ -104,14 +104,14 @@ func (c *kubeConigInfoPublisherController) Sync(ctx controllerlib.Context) error server = *c.serverOverride } - updateServerAndCAFunc := func(c *configv1alpha1.CredentialIssuer) { - c.Status.KubeConfigInfo = &configv1alpha1.CredentialIssuerKubeConfigInfo{ + updateServerAndCAFunc := func(c *configv1alpha1.CredentialIssuerStatus) { + c.KubeConfigInfo = &configv1alpha1.CredentialIssuerKubeConfigInfo{ Server: server, CertificateAuthorityData: certificateAuthorityData, } } - return CreateOrUpdateCredentialIssuer( + return CreateOrUpdateCredentialIssuerStatus( ctx.Context, c.credentialIssuerResourceName, c.credentialIssuerLabels, diff --git a/internal/controller/issuerconfig/kube_config_info_publisher_test.go b/internal/controller/issuerconfig/kube_config_info_publisher_test.go index fd6a6624..bea11703 100644 --- a/internal/controller/issuerconfig/kube_config_info_publisher_test.go +++ b/internal/controller/issuerconfig/kube_config_info_publisher_test.go @@ -115,12 +115,23 @@ func TestSync(t *testing.T) { var timeoutContextCancel context.CancelFunc var syncContext *controllerlib.Context - var expectedCredentialIssuer = func(expectedServerURL, expectedCAData string) (schema.GroupVersionResource, *configv1alpha1.CredentialIssuer) { + var expectedCredentialIssuer = func(expectedServerURL, expectedCAData string) (schema.GroupVersionResource, *configv1alpha1.CredentialIssuer, *configv1alpha1.CredentialIssuer) { expectedCredentialIssuerGVR := schema.GroupVersionResource{ Group: configv1alpha1.GroupName, Version: "v1alpha1", Resource: "credentialissuers", } + + expectedCreateCredentialIssuer := &configv1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{ + Name: credentialIssuerResourceName, + Labels: map[string]string{ + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", + }, + }, + } + expectedCredentialIssuer := &configv1alpha1.CredentialIssuer{ ObjectMeta: metav1.ObjectMeta{ Name: credentialIssuerResourceName, @@ -130,14 +141,13 @@ func TestSync(t *testing.T) { }, }, Status: configv1alpha1.CredentialIssuerStatus{ - Strategies: []configv1alpha1.CredentialIssuerStrategy{}, KubeConfigInfo: &configv1alpha1.CredentialIssuerKubeConfigInfo{ Server: expectedServerURL, CertificateAuthorityData: expectedCAData, }, }, } - return expectedCredentialIssuerGVR, expectedCredentialIssuer + return expectedCredentialIssuerGVR, expectedCreateCredentialIssuer, expectedCredentialIssuer } // Defer starting the informers until the last possible moment so that the @@ -217,16 +227,21 @@ func TestSync(t *testing.T) { err := controllerlib.TestSync(t, subject, *syncContext) r.NoError(err) - expectedCredentialIssuerGVR, expectedCredentialIssuer := expectedCredentialIssuer( + expectedCredentialIssuerGVR, expectedCreateCredentialIssuer, expectedCredentialIssuer := expectedCredentialIssuer( kubeServerURL, caData, ) r.Equal( []coretesting.Action{ - coretesting.NewRootGetAction(expectedCredentialIssuerGVR, expectedCredentialIssuer.Name), + coretesting.NewRootGetAction(expectedCredentialIssuerGVR, expectedCreateCredentialIssuer.Name), coretesting.NewRootCreateAction( expectedCredentialIssuerGVR, + expectedCreateCredentialIssuer, + ), + coretesting.NewRootUpdateSubresourceAction( + expectedCredentialIssuerGVR, + "status", expectedCredentialIssuer, ), }, @@ -261,7 +276,7 @@ func TestSync(t *testing.T) { err := controllerlib.TestSync(t, subject, *syncContext) r.NoError(err) - expectedCredentialIssuerGVR, expectedCredentialIssuer := expectedCredentialIssuer( + expectedCredentialIssuerGVR, expectedCreateCredentialIssuer, expectedCredentialIssuer := expectedCredentialIssuer( kubeServerURL, caData, ) @@ -269,9 +284,14 @@ func TestSync(t *testing.T) { r.Equal( []coretesting.Action{ - coretesting.NewRootGetAction(expectedCredentialIssuerGVR, expectedCredentialIssuer.Name), + coretesting.NewRootGetAction(expectedCredentialIssuerGVR, expectedCreateCredentialIssuer.Name), coretesting.NewRootCreateAction( expectedCredentialIssuerGVR, + expectedCreateCredentialIssuer, + ), + coretesting.NewRootUpdateSubresourceAction( + expectedCredentialIssuerGVR, + "status", expectedCredentialIssuer, ), }, @@ -287,7 +307,7 @@ func TestSync(t *testing.T) { var credentialIssuer *configv1alpha1.CredentialIssuer it.Before(func() { - credentialIssuerGVR, credentialIssuer = expectedCredentialIssuer( + credentialIssuerGVR, _, credentialIssuer = expectedCredentialIssuer( kubeServerURL, caData, ) @@ -311,7 +331,7 @@ func TestSync(t *testing.T) { when("the CredentialIssuer is stale compared to the data in the ConfigMap", func() { it.Before(func() { - _, expectedCredentialIssuer := expectedCredentialIssuer( + _, _, expectedCredentialIssuer := expectedCredentialIssuer( kubeServerURL, caData, ) @@ -324,14 +344,15 @@ func TestSync(t *testing.T) { err := controllerlib.TestSync(t, subject, *syncContext) r.NoError(err) - expectedCredentialIssuerGVR, expectedCredentialIssuer := expectedCredentialIssuer( + expectedCredentialIssuerGVR, _, expectedCredentialIssuer := expectedCredentialIssuer( kubeServerURL, caData, ) expectedActions := []coretesting.Action{ coretesting.NewRootGetAction(expectedCredentialIssuerGVR, expectedCredentialIssuer.Name), - coretesting.NewRootUpdateAction( + coretesting.NewRootUpdateSubresourceAction( expectedCredentialIssuerGVR, + "status", expectedCredentialIssuer, ), } diff --git a/internal/controller/kubecertagent/annotater_test.go b/internal/controller/kubecertagent/annotater_test.go index ea3db3e3..f59fe1a3 100644 --- a/internal/controller/kubecertagent/annotater_test.go +++ b/internal/controller/kubecertagent/annotater_test.go @@ -265,8 +265,9 @@ func TestAnnotaterControllerSync(t *testing.T) { credentialIssuerGVR, credentialIssuerResourceName, ) - expectedUpdateAction := coretesting.NewRootUpdateAction( + expectedUpdateAction := coretesting.NewRootUpdateSubresourceAction( credentialIssuerGVR, + "status", expectedCredentialIssuer, ) @@ -304,6 +305,13 @@ func TestAnnotaterControllerSync(t *testing.T) { startInformersAndController() err := controllerlib.TestSync(t, subject, *syncContext) + expectedCreateCredentialIssuer := &configv1alpha1.CredentialIssuer{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: credentialIssuerResourceName, + }, + } + expectedCredentialIssuer := &configv1alpha1.CredentialIssuer{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ @@ -327,6 +335,11 @@ func TestAnnotaterControllerSync(t *testing.T) { ) expectedCreateAction := coretesting.NewRootCreateAction( credentialIssuerGVR, + expectedCreateCredentialIssuer, + ) + expectedUpdateAction := coretesting.NewRootUpdateSubresourceAction( + credentialIssuerGVR, + "status", expectedCredentialIssuer, ) @@ -335,6 +348,7 @@ func TestAnnotaterControllerSync(t *testing.T) { []coretesting.Action{ expectedGetAction, expectedCreateAction, + expectedUpdateAction, }, pinnipedAPIClient.Actions(), ) diff --git a/internal/controller/kubecertagent/creater_test.go b/internal/controller/kubecertagent/creater_test.go index d63363c1..e9592519 100644 --- a/internal/controller/kubecertagent/creater_test.go +++ b/internal/controller/kubecertagent/creater_test.go @@ -336,8 +336,9 @@ func TestCreaterControllerSync(t *testing.T) { credentialIssuerGVR, credentialIssuerResourceName, ) - expectedUpdateAction := coretesting.NewRootUpdateAction( + expectedUpdateAction := coretesting.NewRootUpdateSubresourceAction( credentialIssuerGVR, + "status", expectedCredentialIssuer, ) @@ -375,6 +376,17 @@ func TestCreaterControllerSync(t *testing.T) { startInformersAndController() err := controllerlib.TestSync(t, subject, *syncContext) + expectedCreateCredentialIssuer := &configv1alpha1.CredentialIssuer{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: credentialIssuerResourceName, + Labels: map[string]string{ + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", + }, + }, + } + expectedCredentialIssuer := &configv1alpha1.CredentialIssuer{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ @@ -402,6 +414,11 @@ func TestCreaterControllerSync(t *testing.T) { ) expectedCreateAction := coretesting.NewRootCreateAction( credentialIssuerGVR, + expectedCreateCredentialIssuer, + ) + expectedUpdateAction := coretesting.NewRootUpdateSubresourceAction( + credentialIssuerGVR, + "status", expectedCredentialIssuer, ) @@ -410,6 +427,7 @@ func TestCreaterControllerSync(t *testing.T) { []coretesting.Action{ expectedGetAction, expectedCreateAction, + expectedUpdateAction, }, pinnipedAPIClient.Actions(), ) @@ -458,8 +476,9 @@ func TestCreaterControllerSync(t *testing.T) { credentialIssuerGVR, credentialIssuerResourceName, ) - expectedUpdateAction := coretesting.NewRootUpdateAction( + expectedUpdateAction := coretesting.NewRootUpdateSubresourceAction( credentialIssuerGVR, + "status", expectedCredentialIssuer, ) @@ -514,6 +533,17 @@ func TestCreaterControllerSync(t *testing.T) { startInformersAndController() err := controllerlib.TestSync(t, subject, *syncContext) + expectedCreateCredentialIssuer := &configv1alpha1.CredentialIssuer{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: credentialIssuerResourceName, + Labels: map[string]string{ + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", + }, + }, + } + expectedCredentialIssuer := &configv1alpha1.CredentialIssuer{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ @@ -541,6 +571,11 @@ func TestCreaterControllerSync(t *testing.T) { ) expectedCreateAction := coretesting.NewRootCreateAction( credentialIssuerGVR, + expectedCreateCredentialIssuer, + ) + expectedUpdateAction := coretesting.NewRootUpdateSubresourceAction( + credentialIssuerGVR, + "status", expectedCredentialIssuer, ) @@ -549,6 +584,7 @@ func TestCreaterControllerSync(t *testing.T) { []coretesting.Action{ expectedGetAction, expectedCreateAction, + expectedUpdateAction, }, pinnipedAPIClient.Actions(), ) diff --git a/internal/controller/kubecertagent/execer_test.go b/internal/controller/kubecertagent/execer_test.go index e7ae425a..5b3d8d58 100644 --- a/internal/controller/kubecertagent/execer_test.go +++ b/internal/controller/kubecertagent/execer_test.go @@ -358,7 +358,7 @@ func TestManagerControllerSync(t *testing.T) { }, } expectedGetAction := coretesting.NewRootGetAction(credentialIssuerGVR, credentialIssuerResourceName) - expectedCreateAction := coretesting.NewRootUpdateAction(credentialIssuerGVR, expectedCredentialIssuer) + expectedCreateAction := coretesting.NewRootUpdateSubresourceAction(credentialIssuerGVR, "status", expectedCredentialIssuer) r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction}, pinnipedAPIClient.Actions()) }) @@ -389,6 +389,13 @@ func TestManagerControllerSync(t *testing.T) { it("also creates the the CredentialIssuer with the appropriate status field", func() { r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + expectedCreateCredentialIssuer := &configv1alpha1.CredentialIssuer{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: credentialIssuerResourceName, + }, + } + expectedCredentialIssuer := &configv1alpha1.CredentialIssuer{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ @@ -407,8 +414,9 @@ func TestManagerControllerSync(t *testing.T) { }, } expectedGetAction := coretesting.NewRootGetAction(credentialIssuerGVR, credentialIssuerResourceName) - expectedCreateAction := coretesting.NewRootCreateAction(credentialIssuerGVR, expectedCredentialIssuer) - r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction}, pinnipedAPIClient.Actions()) + expectedCreateAction := coretesting.NewRootCreateAction(credentialIssuerGVR, expectedCreateCredentialIssuer) + expectedUpdateAction := coretesting.NewRootUpdateSubresourceAction(credentialIssuerGVR, "status", expectedCredentialIssuer) + r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction, expectedUpdateAction}, pinnipedAPIClient.Actions()) }) }) }) @@ -431,6 +439,13 @@ func TestManagerControllerSync(t *testing.T) { it("creates or updates the the CredentialIssuer status field with an error", func() { r.EqualError(controllerlib.TestSync(t, subject, *syncContext), podExecErrorMessage) + expectedCreateCredentialIssuer := &configv1alpha1.CredentialIssuer{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: credentialIssuerResourceName, + }, + } + expectedCredentialIssuer := &configv1alpha1.CredentialIssuer{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ @@ -449,8 +464,9 @@ func TestManagerControllerSync(t *testing.T) { }, } expectedGetAction := coretesting.NewRootGetAction(credentialIssuerGVR, credentialIssuerResourceName) - expectedCreateAction := coretesting.NewRootCreateAction(credentialIssuerGVR, expectedCredentialIssuer) - r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction}, pinnipedAPIClient.Actions()) + expectedCreateAction := coretesting.NewRootCreateAction(credentialIssuerGVR, expectedCreateCredentialIssuer) + expectedUpdateAction := coretesting.NewRootUpdateSubresourceAction(credentialIssuerGVR, "status", expectedCredentialIssuer) + r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction, expectedUpdateAction}, pinnipedAPIClient.Actions()) }) }) @@ -472,6 +488,13 @@ func TestManagerControllerSync(t *testing.T) { it("creates or updates the the CredentialIssuer status field with an error", func() { r.EqualError(controllerlib.TestSync(t, subject, *syncContext), podExecErrorMessage) + expectedCreateCredentialIssuer := &configv1alpha1.CredentialIssuer{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: credentialIssuerResourceName, + }, + } + expectedCredentialIssuer := &configv1alpha1.CredentialIssuer{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ @@ -490,8 +513,9 @@ func TestManagerControllerSync(t *testing.T) { }, } expectedGetAction := coretesting.NewRootGetAction(credentialIssuerGVR, credentialIssuerResourceName) - expectedCreateAction := coretesting.NewRootCreateAction(credentialIssuerGVR, expectedCredentialIssuer) - r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction}, pinnipedAPIClient.Actions()) + expectedCreateAction := coretesting.NewRootCreateAction(credentialIssuerGVR, expectedCreateCredentialIssuer) + expectedUpdateAction := coretesting.NewRootUpdateSubresourceAction(credentialIssuerGVR, "status", expectedCredentialIssuer) + r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction, expectedUpdateAction}, pinnipedAPIClient.Actions()) }) }) }) diff --git a/internal/controller/kubecertagent/kubecertagent.go b/internal/controller/kubecertagent/kubecertagent.go index 1dfe7a67..0122a5d4 100644 --- a/internal/controller/kubecertagent/kubecertagent.go +++ b/internal/controller/kubecertagent/kubecertagent.go @@ -287,19 +287,19 @@ func createOrUpdateCredentialIssuer(ctx context.Context, pinnipedAPIClient pinnipedclientset.Interface, err error, ) error { - return issuerconfig.CreateOrUpdateCredentialIssuer( + return issuerconfig.CreateOrUpdateCredentialIssuerStatus( ctx, ciConfig.Name, credentialIssuerLabels, pinnipedAPIClient, - func(configToUpdate *configv1alpha1.CredentialIssuer) { + func(configToUpdate *configv1alpha1.CredentialIssuerStatus) { var strategyResult configv1alpha1.CredentialIssuerStrategy if err == nil { strategyResult = strategySuccess(clock) } else { strategyResult = strategyError(clock, err) } - configToUpdate.Status.Strategies = []configv1alpha1.CredentialIssuerStrategy{ + configToUpdate.Strategies = []configv1alpha1.CredentialIssuerStrategy{ strategyResult, } }, diff --git a/test/integration/kubeclient_test.go b/test/integration/kubeclient_test.go index 0448c4f6..dbe5c9c4 100644 --- a/test/integration/kubeclient_test.go +++ b/test/integration/kubeclient_test.go @@ -220,9 +220,6 @@ func TestKubeClientOwnerRef(t *testing.T) { GenerateName: "owner-ref-test-", OwnerReferences: nil, // no owner refs set }, - Status: conciergeconfigv1alpha1.CredentialIssuerStatus{ - Strategies: []conciergeconfigv1alpha1.CredentialIssuerStrategy{}, - }, }, metav1.CreateOptions{}, )