diff --git a/internal/controller/impersonatorconfig/impersonator_config.go b/internal/controller/impersonatorconfig/impersonator_config.go index 6c93b202..3fdbed78 100644 --- a/internal/controller/impersonatorconfig/impersonator_config.go +++ b/internal/controller/impersonatorconfig/impersonator_config.go @@ -21,6 +21,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/clock" "k8s.io/apimachinery/pkg/util/errors" + utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/intstr" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/sets" @@ -152,8 +153,13 @@ func NewImpersonatorConfigController( func (c *impersonatorConfigController) Sync(syncCtx controllerlib.Context) error { plog.Debug("Starting impersonatorConfigController Sync") - strategy, err := c.doSync(syncCtx) + // Load the CredentialIssuer that we'll update with status. + credIssuer, err := c.credIssuerInformer.Lister().Get(c.credentialIssuerResourceName) + if err != nil { + return fmt.Errorf("could not get CredentialIssuer to update: %w", err) + } + strategy, err := c.doSync(syncCtx, credIssuer) if err != nil { strategy = &v1alpha1.CredentialIssuerStrategy{ Type: v1alpha1.ImpersonationProxyStrategyType, @@ -166,13 +172,12 @@ func (c *impersonatorConfigController) Sync(syncCtx controllerlib.Context) error c.clearSignerCA() } - updateStrategyErr := c.updateStrategy(syncCtx.Context, strategy) - if updateStrategyErr != nil { - plog.Error("error while updating the CredentialIssuer status", err) - if err == nil { - err = updateStrategyErr - } - } + err = utilerrors.NewAggregate([]error{err, issuerconfig.Update( + syncCtx.Context, + c.pinnipedAPIClient, + credIssuer, + *strategy, + )}) if err == nil { plog.Debug("Successfully finished impersonatorConfigController Sync") @@ -196,10 +201,10 @@ type certNameInfo struct { clientEndpoint string } -func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context) (*v1alpha1.CredentialIssuerStrategy, error) { +func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context, credIssuer *v1alpha1.CredentialIssuer) (*v1alpha1.CredentialIssuerStrategy, error) { ctx := syncCtx.Context - config, err := c.loadImpersonationProxyConfiguration() + impersonationSpec, err := c.loadImpersonationProxyConfiguration(credIssuer) if err != nil { return nil, err } @@ -217,7 +222,7 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context) (*v plog.Debug("Queried for control plane nodes", "foundControlPlaneNodes", hasControlPlaneNodes) } - if c.shouldHaveImpersonator(config) { + if c.shouldHaveImpersonator(impersonationSpec) { if err = c.ensureImpersonatorIsStarted(syncCtx); err != nil { return nil, err } @@ -227,8 +232,8 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context) (*v } } - if c.shouldHaveLoadBalancer(config) { - if err = c.ensureLoadBalancerIsStarted(ctx, config); err != nil { + if c.shouldHaveLoadBalancer(impersonationSpec) { + if err = c.ensureLoadBalancerIsStarted(ctx, impersonationSpec); err != nil { return nil, err } } else { @@ -237,8 +242,8 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context) (*v } } - if c.shouldHaveClusterIPService(config) { - if err = c.ensureClusterIPServiceIsStarted(ctx, config); err != nil { + if c.shouldHaveClusterIPService(impersonationSpec) { + if err = c.ensureClusterIPServiceIsStarted(ctx, impersonationSpec); err != nil { return nil, err } } // else { // TODO test stopping the cluster ip service @@ -247,7 +252,7 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context) (*v // } //} - nameInfo, err := c.findDesiredTLSCertificateName(config) + nameInfo, err := c.findDesiredTLSCertificateName(impersonationSpec) if err != nil { // Unexpected error while determining the name that should go into the certs, so clear any existing certs. c.tlsServingCertDynamicCertProvider.UnsetCertKeyContent() @@ -255,7 +260,7 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context) (*v } var impersonationCA *certauthority.CA - if c.shouldHaveTLSSecret(config) { + if c.shouldHaveTLSSecret(impersonationSpec) { if impersonationCA, err = c.ensureCASecretIsCreated(ctx); err != nil { return nil, err } @@ -266,7 +271,7 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context) (*v return nil, err } - credentialIssuerStrategyResult := c.doSyncResult(nameInfo, config, impersonationCA) + credentialIssuerStrategyResult := c.doSyncResult(nameInfo, impersonationSpec, impersonationCA) if err = c.loadSignerCA(credentialIssuerStrategyResult.Status); err != nil { return nil, err @@ -275,12 +280,7 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context) (*v return credentialIssuerStrategyResult, nil } -func (c *impersonatorConfigController) loadImpersonationProxyConfiguration() (*v1alpha1.ImpersonationProxySpec, error) { - credIssuer, err := c.credIssuerInformer.Lister().Get(c.credentialIssuerResourceName) - if err != nil { - return nil, fmt.Errorf("failed to get %s CredentialIssuer: %w", c.credentialIssuerResourceName, err) - } - +func (c *impersonatorConfigController) loadImpersonationProxyConfiguration(credIssuer *v1alpha1.CredentialIssuer) (*v1alpha1.ImpersonationProxySpec, error) { // Make a copy of the spec since we got this object from informer cache. spec := credIssuer.Spec.DeepCopy().ImpersonationProxy if spec == nil { @@ -329,11 +329,6 @@ func (c *impersonatorConfigController) shouldHaveTLSSecret(config *v1alpha1.Impe return c.shouldHaveImpersonator(config) } -func (c *impersonatorConfigController) updateStrategy(ctx context.Context, strategy *v1alpha1.CredentialIssuerStrategy) error { - // TODO use informer client rather than api client for reading - return issuerconfig.UpdateStrategy(ctx, c.credentialIssuerResourceName, c.labels, c.pinnipedAPIClient, *strategy) -} - func (c *impersonatorConfigController) loadBalancerExists() (bool, error) { _, err := c.servicesInformer.Lister().Services(c.namespace).Get(c.generatedLoadBalancerServiceName) notFound := k8serrors.IsNotFound(err) diff --git a/internal/controller/impersonatorconfig/impersonator_config_test.go b/internal/controller/impersonatorconfig/impersonator_config_test.go index 87ac1495..db9b06ea 100644 --- a/internal/controller/impersonatorconfig/impersonator_config_test.go +++ b/internal/controller/impersonatorconfig/impersonator_config_test.go @@ -554,14 +554,10 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { controllerlib.TestRunSynchronously(t, subject) } - var addCredentialIssuerToTracker = func(resourceName string, credIssuerSpec v1alpha1.CredentialIssuerSpec, client *pinnipedfake.Clientset) { - t.Logf("adding CredentialIssuer %s to informer clientset", resourceName) - r.NoError(client.Tracker().Add(&v1alpha1.CredentialIssuer{ - ObjectMeta: metav1.ObjectMeta{ - Name: resourceName, - }, - Spec: credIssuerSpec, - })) + var addCredentialIssuerToTrackers = func(credIssuer v1alpha1.CredentialIssuer, informerClient *pinnipedfake.Clientset, mainClient *pinnipedfake.Clientset) { + t.Logf("adding CredentialIssuer %s to informer and main clientsets", credIssuer.Name) + r.NoError(informerClient.Tracker().Add(&credIssuer)) + r.NoError(mainClient.Tracker().Add(&credIssuer)) } var newSecretWithData = func(resourceName string, data map[string][]byte) *corev1.Secret { @@ -856,7 +852,6 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { // As long as we get the final result that we wanted then we are happy for the purposes // of this test. credentialIssuer := getCredentialIssuer() - r.Equal(labels, credentialIssuer.Labels) r.Equal([]v1alpha1.CredentialIssuerStrategy{expectedStrategy}, credentialIssuer.Status.Strategies) } @@ -1023,7 +1018,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("errors and does nothing else", func() { startInformersAndController() - r.EqualError(runControllerSync(), "failed to get some-credential-issuer-resource-name CredentialIssuer: credentialissuer.config.concierge.pinniped.dev \"some-credential-issuer-resource-name\" not found") + r.EqualError(runControllerSync(), `could not get CredentialIssuer to update: credentialissuer.config.concierge.pinniped.dev "some-credential-issuer-resource-name" not found`) requireTLSServerWasNeverStarted() r.Len(kubeAPIClient.Actions(), 0) }) @@ -1033,15 +1028,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the configuration is auto mode with an endpoint and service type none", func() { it.Before(func() { addSecretToTrackers(signingCASecret, kubeInformerClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeAuto, - ExternalEndpoint: localhostIP, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeNone, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeAuto, + ExternalEndpoint: localhostIP, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) }) when("there are visible control plane nodes", func() { @@ -1082,11 +1080,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the configuration is auto mode", func() { it.Before(func() { addSecretToTrackers(signingCASecret, kubeInformerClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeAuto, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeAuto, + }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) }) when("there are visible control plane nodes", func() { @@ -1380,11 +1381,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the configuration is disabled mode", func() { it.Before(func() { addSecretToTrackers(signingCASecret, kubeInformerClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeDisabled, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeDisabled, + }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) }) @@ -1405,11 +1409,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { }) when("no load balancer", func() { it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("control-plane", kubeAPIClient) }) @@ -1436,11 +1443,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("a loadbalancer already exists", func() { it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) addLoadBalancerServiceToTracker(loadBalancerServiceName, kubeInformerClient) addLoadBalancerServiceToTracker(loadBalancerServiceName, kubeAPIClient) @@ -1469,11 +1479,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("a load balancer and a secret already exists", func() { var caCrt []byte it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) ca := newCA() caSecret := newActualCASecret(ca, caSecretName) @@ -1499,15 +1512,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("credentialissuer has service type loadbalancer and custom annotations", func() { annotations := map[string]string{"some-annotation-key": "some-annotation-value"} it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeLoadBalancer, - Annotations: annotations, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeLoadBalancer, + Annotations: annotations, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) }) @@ -1528,15 +1544,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the CredentialIssuer has a hostname specified and service type none", func() { const fakeHostname = "fake.example.com" it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: fakeHostname, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeNone, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: fakeHostname, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) }) @@ -1557,15 +1576,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the CredentialIssuer has a hostname specified and service type loadbalancer", func() { const fakeHostname = "fake.example.com" it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: fakeHostname, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeLoadBalancer, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: fakeHostname, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeLoadBalancer, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) }) @@ -1586,14 +1608,17 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the CredentialIssuer has a hostname specified and service type clusterip", func() { it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeClusterIP, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeClusterIP, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) }) @@ -1614,15 +1639,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the CredentialIssuer has a endpoint which is an IP address with a port", func() { const fakeIPWithPort = "127.0.0.1:3000" it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: fakeIPWithPort, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeNone, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: fakeIPWithPort, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) }) @@ -1643,15 +1671,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the CredentialIssuer has a endpoint which is a hostname with a port, service type none", func() { const fakeHostnameWithPort = "fake.example.com:3000" it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: fakeHostnameWithPort, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeNone, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: fakeHostnameWithPort, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) }) @@ -1672,16 +1703,19 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the CredentialIssuer has a endpoint which is a hostname with a port, service type loadbalancer with loadbalancerip", func() { const fakeHostnameWithPort = "fake.example.com:3000" it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: fakeHostnameWithPort, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeLoadBalancer, - LoadBalancerIP: localhostIP, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: fakeHostnameWithPort, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeLoadBalancer, + LoadBalancerIP: localhostIP, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) }) @@ -1725,7 +1759,10 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { } it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, ipAddressConfig, pinnipedInformerClient) + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: ipAddressConfig, + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) }) @@ -1779,15 +1816,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the TLS cert goes missing and needs to be recreated, e.g. when a user manually deleted it", func() { const fakeHostname = "fake.example.com" it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: fakeHostname, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeNone, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: fakeHostname, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) startInformersAndController() }) @@ -1824,15 +1864,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the CA cert goes missing and needs to be recreated, e.g. when a user manually deleted it", func() { const fakeHostname = "fake.example.com" it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: fakeHostname, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeNone, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: fakeHostname, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) startInformersAndController() }) @@ -1872,15 +1915,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { const fakeHostname = "fake.example.com" var caCrt []byte it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: fakeHostname, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeNone, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: fakeHostname, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) startInformersAndController() r.NoError(runControllerSync()) @@ -1944,11 +1990,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the configuration switches from enabled to disabled mode", func() { it.Before(func() { addSecretToTrackers(signingCASecret, kubeInformerClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) }) @@ -2004,15 +2053,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the endpoint and mode switch from specified with no service, to not specified, to specified again", func() { it.Before(func() { addSecretToTrackers(signingCASecret, kubeInformerClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: localhostIP, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeNone, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: localhostIP, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) }) @@ -2099,15 +2151,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("requesting a load balancer via CredentialIssuer, then updating the annotations", func() { it.Before(func() { addSecretToTrackers(signingCASecret, kubeInformerClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: localhostIP, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeLoadBalancer, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: localhostIP, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeLoadBalancer, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) }) @@ -2157,15 +2212,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("requesting a load balancer via CredentialIssuer, then adding a static loadBalancerIP to the spec", func() { it.Before(func() { addSecretToTrackers(signingCASecret, kubeInformerClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: localhostIP, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeLoadBalancer, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: localhostIP, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeLoadBalancer, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) }) @@ -2217,11 +2275,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it.Before(func() { addSecretToTrackers(signingCASecret, kubeInformerClient) addNodeWithRoleToTracker("worker", kubeAPIClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeAuto, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeAuto, + }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) }) it("only starts the impersonator once and only lists the cluster's nodes once", func() { @@ -2335,15 +2396,19 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it.Before(func() { addSecretToTrackers(signingCASecret, kubeInformerClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeAuto, - }, - }, pinnipedInformerClient) - r.NoError(pinnipedAPIClient.Tracker().Add(&v1alpha1.CredentialIssuer{ + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, - Status: v1alpha1.CredentialIssuerStatus{Strategies: []v1alpha1.CredentialIssuerStrategy{preExistingStrategy}}, - })) + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeAuto, + }, + }, + Status: v1alpha1.CredentialIssuerStatus{ + Strategies: []v1alpha1.CredentialIssuerStrategy{ + preExistingStrategy, + }, + }, + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) }) @@ -2362,11 +2427,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("getting the control plane nodes returns an error, e.g. when there are no nodes", func() { it("returns an error", func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeAuto, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeAuto, + }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) startInformersAndController() r.EqualError(runControllerSync(), "no nodes found") requireCredentialIssuer(newErrorStrategy("no nodes found")) @@ -2379,11 +2447,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it.Before(func() { addNodeWithRoleToTracker("worker", kubeAPIClient) impersonatorFuncReturnedFuncError = errors.New("some immediate impersonator startup error") - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeAuto, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeAuto, + }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) }) it("causes an immediate resync, returns an error on that next sync, and then restarts the server in a following sync", func() { @@ -2439,11 +2510,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the impersonator server dies for no apparent reason after running for a while", func() { it.Before(func() { addNodeWithRoleToTracker("worker", kubeAPIClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeAuto, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeAuto, + }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) }) it("causes an immediate resync, returns an error on that next sync, and then restarts the server in a following sync", func() { @@ -2495,9 +2569,12 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the CredentialIssuer has nil impersonation spec", func() { it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: nil, - }, pinnipedInformerClient) + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: nil, + }, + }, pinnipedInformerClient, pinnipedAPIClient) }) it("returns an error", func() { @@ -2512,11 +2589,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the CredentialIssuer has invalid mode", func() { it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: "not-valid", + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: "not-valid", + }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) }) it("returns an error", func() { @@ -2531,14 +2611,17 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the CredentialIssuer has invalid service type", func() { it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: "not-valid", + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: "not-valid", + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) }) it("returns an error", func() { @@ -2553,14 +2636,17 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the CredentialIssuer has invalid LoadBalancerIP", func() { it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - LoadBalancerIP: "invalid-ip-address", + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + LoadBalancerIP: "invalid-ip-address", + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) }) it("returns an error", func() { @@ -2575,12 +2661,15 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the CredentialIssuer has invalid ExternalEndpoint", func() { it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: "[invalid", + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: "[invalid", + }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) }) it("returns an error", func() { @@ -2599,11 +2688,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { kubeAPIClient.PrependReactor("create", "services", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) { return true, nil, fmt.Errorf("error on create") }) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeAuto, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeAuto, + }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) }) it("returns an error", func() { @@ -2617,15 +2709,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("there is an error creating the tls secret", func() { it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: "example.com", - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeNone, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: "example.com", + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("control-plane", kubeAPIClient) kubeAPIClient.PrependReactor("create", "secrets", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) { createdSecret := action.(coretesting.CreateAction).GetObject().(*corev1.Secret) @@ -2651,15 +2746,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("there is an error creating the CA secret", func() { it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: "example.com", - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeNone, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: "example.com", + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("control-plane", kubeAPIClient) kubeAPIClient.PrependReactor("create", "secrets", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) { createdSecret := action.(coretesting.CreateAction).GetObject().(*corev1.Secret) @@ -2685,15 +2783,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the CA secret exists but is invalid while the TLS secret needs to be created", func() { it.Before(func() { addNodeWithRoleToTracker("control-plane", kubeAPIClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: "example.com", - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeNone, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: "example.com", + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addSecretToTrackers(newEmptySecret(caSecretName), kubeAPIClient, kubeInformerClient) }) @@ -2715,11 +2816,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { addLoadBalancerServiceToTracker(loadBalancerServiceName, kubeInformerClient) addLoadBalancerServiceToTracker(loadBalancerServiceName, kubeAPIClient) addSecretToTrackers(newEmptySecret(tlsSecretName), kubeAPIClient, kubeInformerClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeAuto, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeAuto, + }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) startInformersAndController() kubeAPIClient.PrependReactor("delete", "secrets", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) { return true, nil, fmt.Errorf("error on delete") @@ -2742,11 +2846,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it.Before(func() { addNodeWithRoleToTracker("control-plane", kubeAPIClient) addSecretToTrackers(newEmptySecret(tlsSecretName), kubeInformerClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeDisabled, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeDisabled, + }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) }) it("does not pass the not found error through", func() { @@ -2764,15 +2871,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the PEM formatted data in the TLS Secret is not a valid cert", func() { it.Before(func() { addSecretToTrackers(signingCASecret, kubeInformerClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: localhostIP, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeNone, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: localhostIP, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) tlsSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -2827,11 +2937,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { var caCrt []byte it.Before(func() { addSecretToTrackers(signingCASecret, kubeInformerClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) ca := newCA() caSecret := newActualCASecret(ca, caSecretName) @@ -2890,11 +3003,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { addSecretToTrackers(tlsSecret, kubeAPIClient, kubeInformerClient) addLoadBalancerServiceWithIngressToTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: localhostIP}}, kubeInformerClient) addLoadBalancerServiceWithIngressToTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: localhostIP}}, kubeAPIClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeAuto, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeAuto, + }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) }) it("deletes the invalid certs, creates new certs, and starts the impersonator", func() { @@ -2935,19 +3051,22 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it.Before(func() { addSecretToTrackers(signingCASecret, kubeInformerClient) addNodeWithRoleToTracker("worker", kubeAPIClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeAuto, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeAuto, + }, }, - }, pinnipedInformerClient) - pinnipedAPIClient.PrependReactor("create", "credentialissuers", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) { - return true, nil, fmt.Errorf("error on create") + }, pinnipedInformerClient, pinnipedAPIClient) + pinnipedAPIClient.PrependReactor("update", "credentialissuers", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, fmt.Errorf("error on update") }) }) it("returns the error", func() { startInformersAndController() - r.EqualError(runControllerSync(), "could not create or update credentialissuer: create failed: error on create") + r.EqualError(runControllerSync(), "failed to update CredentialIssuer status: error on update") }) when("there is also a more fundamental error while starting the impersonator", func() { @@ -2957,9 +3076,9 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { }) }) - it("returns the more fundamental error instead of the CredentialIssuer error", func() { + it("returns both errors", func() { startInformersAndController() - r.EqualError(runControllerSync(), "error on service creation") + r.EqualError(runControllerSync(), "[error on service creation, failed to update CredentialIssuer status: error on update]") }) }) }) @@ -2967,15 +3086,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the impersonator is ready but there is a problem with the signing secret, which should be created by another controller", func() { const fakeHostname = "foo.example.com" it.Before(func() { - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: fakeHostname, - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeNone, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: fakeHostname, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) }) @@ -3059,15 +3181,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the impersonator is enabled but the service type is none and the external endpoint is empty", func() { it.Before(func() { addSecretToTrackers(signingCASecret, kubeInformerClient) - addCredentialIssuerToTracker(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ - ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ - Mode: v1alpha1.ImpersonationProxyModeEnabled, - ExternalEndpoint: "", - Service: v1alpha1.ImpersonationProxyServiceSpec{ - Type: v1alpha1.ImpersonationProxyServiceTypeNone, + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: "", + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, }, }, - }, pinnipedInformerClient) + }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("control-plane", kubeAPIClient) }) @@ -3078,7 +3203,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { }) }) }) - }, spec.Report(report.Terminal{})) + }, spec.Report(report.Terminal{})) // TODO: replace the Parallel() call here } type testQueue struct { diff --git a/internal/controller/issuerconfig/create_or_update_credential_issuer_config.go b/internal/controller/issuerconfig/create_or_update_credential_issuer_config.go deleted file mode 100644 index 7d936b3a..00000000 --- a/internal/controller/issuerconfig/create_or_update_credential_issuer_config.go +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -package issuerconfig - -import ( - "context" - "fmt" - - "k8s.io/apimachinery/pkg/api/equality" - - k8serrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/util/retry" - - configv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/config/v1alpha1" - pinnipedclientset "go.pinniped.dev/generated/latest/client/concierge/clientset/versioned" -) - -func CreateOrUpdateCredentialIssuerStatus( - ctx context.Context, - credentialIssuerResourceName string, - credentialIssuerLabels map[string]string, - pinnipedClient pinnipedclientset.Interface, - applyUpdatesToCredentialIssuerFunc func(configToUpdate *configv1alpha1.CredentialIssuerStatus), -) error { - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - existingCredentialIssuer, err := pinnipedClient. - ConfigV1alpha1(). - CredentialIssuers(). - Get(ctx, credentialIssuerResourceName, metav1.GetOptions{}) - - notFound := k8serrors.IsNotFound(err) - if err != nil && !notFound { - return fmt.Errorf("get failed: %w", err) - } - - credentialIssuersClient := pinnipedClient.ConfigV1alpha1().CredentialIssuers() - - if notFound { - // create an empty credential issuer - minCredentialIssuer := minimalValidCredentialIssuer(credentialIssuerResourceName, credentialIssuerLabels) - - newCredentialIssuer, err := credentialIssuersClient.Create(ctx, minCredentialIssuer, metav1.CreateOptions{}) - if err != nil { - return fmt.Errorf("create failed: %w", 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 - }) - - if err != nil { - return fmt.Errorf("could not create or update credentialissuer: %w", err) - } - return nil -} - -func minimalValidCredentialIssuer( - credentialIssuerName string, - credentialIssuerLabels map[string]string, -) *configv1alpha1.CredentialIssuer { - return &configv1alpha1.CredentialIssuer{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - Name: credentialIssuerName, - Labels: credentialIssuerLabels, - }, - } -} 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 deleted file mode 100644 index beb20e56..00000000 --- a/internal/controller/issuerconfig/create_or_update_credential_issuer_config_test.go +++ /dev/null @@ -1,301 +0,0 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -package issuerconfig - -import ( - "context" - "fmt" - "testing" - "time" - - "github.com/sclevine/spec" - "github.com/sclevine/spec/report" - "github.com/stretchr/testify/require" - apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - coretesting "k8s.io/client-go/testing" - apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" - - configv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/config/v1alpha1" - pinnipedfake "go.pinniped.dev/generated/latest/client/concierge/clientset/versioned/fake" -) - -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 - var pinnipedAPIClient *pinnipedfake.Clientset - var credentialIssuerGVR schema.GroupVersionResource - const credentialIssuerResourceName = "some-resource-name" - - it.Before(func() { - r = require.New(t) - ctx = context.Background() - pinnipedAPIClient = pinnipedfake.NewSimpleClientset() - credentialIssuerGVR = schema.GroupVersionResource{ - Group: configv1alpha1.GroupName, - Version: configv1alpha1.SchemeGroupVersion.Version, - Resource: "credentialissuers", - } - }) - - when("the config does not exist", func() { - it("creates a new config and then updates it with the func parameter", func() { - err := CreateOrUpdateCredentialIssuerStatus( - ctx, - credentialIssuerResourceName, - map[string]string{ - "myLabelKey1": "myLabelValue1", - "myLabelKey2": "myLabelValue2", - }, - pinnipedAPIClient, - func(configToUpdate *configv1alpha1.CredentialIssuerStatus) { - configToUpdate.KubeConfigInfo = &configv1alpha1.CredentialIssuerKubeConfigInfo{ - CertificateAuthorityData: "some-ca-value", - } - }, - ) - r.NoError(err) - - expectedGetAction := coretesting.NewRootGetAction(credentialIssuerGVR, credentialIssuerResourceName) - - 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{ - Name: credentialIssuerResourceName, - Labels: map[string]string{ - "myLabelKey1": "myLabelValue1", - "myLabelKey2": "myLabelValue2", - }, - }, - Status: configv1alpha1.CredentialIssuerStatus{ - KubeConfigInfo: &configv1alpha1.CredentialIssuerKubeConfigInfo{ - Server: "", - CertificateAuthorityData: "some-ca-value", - }, - }, - }, - ) - - r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction, expectedUpdateAction}, pinnipedAPIClient.Actions()) - }) - - when("there is an unexpected error while creating the existing object", func() { - it.Before(func() { - pinnipedAPIClient.PrependReactor("create", "credentialissuers", func(_ coretesting.Action) (bool, runtime.Object, error) { - return true, nil, fmt.Errorf("error on create") - }) - }) - - it("returns an error", func() { - err := CreateOrUpdateCredentialIssuerStatus( - ctx, - credentialIssuerResourceName, - map[string]string{}, - pinnipedAPIClient, - func(configToUpdate *configv1alpha1.CredentialIssuerStatus) {}, - ) - r.EqualError(err, "could not create or update credentialissuer: create failed: error on create") - }) - }) - }) - - when("the config already exists", func() { - var existingConfig *configv1alpha1.CredentialIssuer - - it.Before(func() { - existingConfig = &configv1alpha1.CredentialIssuer{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - Name: credentialIssuerResourceName, - Labels: map[string]string{ - "myLabelKey1": "myLabelValue1", - }, - }, - Status: configv1alpha1.CredentialIssuerStatus{ - Strategies: []configv1alpha1.CredentialIssuerStrategy{ - { - Type: configv1alpha1.KubeClusterSigningCertificateStrategyType, - Status: configv1alpha1.SuccessStrategyStatus, - Reason: configv1alpha1.FetchedKeyStrategyReason, - Message: "initial-message", - LastUpdateTime: metav1.Now(), - }, - }, - KubeConfigInfo: &configv1alpha1.CredentialIssuerKubeConfigInfo{ - Server: "initial-server-value", - CertificateAuthorityData: "initial-ca-value", - }, - }, - } - r.NoError(pinnipedAPIClient.Tracker().Add(existingConfig)) - }) - - it("updates the existing config to only apply the updates made by the func parameter", func() { - err := CreateOrUpdateCredentialIssuerStatus( - ctx, - credentialIssuerResourceName, - map[string]string{ - "myLabelKey1": "myLabelValue1", - "myLabelKey2": "myLabelValue2", - }, - pinnipedAPIClient, - func(configToUpdate *configv1alpha1.CredentialIssuerStatus) { - configToUpdate.KubeConfigInfo.CertificateAuthorityData = "new-ca-value" - }, - ) - r.NoError(err) - - expectedGetAction := coretesting.NewRootGetAction(credentialIssuerGVR, credentialIssuerResourceName) - - // Only the edited field should be changed. - expectedUpdatedConfig := existingConfig.DeepCopy() - expectedUpdatedConfig.Status.KubeConfigInfo.CertificateAuthorityData = "new-ca-value" - 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 := CreateOrUpdateCredentialIssuerStatus( - ctx, - credentialIssuerResourceName, - map[string]string{}, - pinnipedAPIClient, - func(configToUpdate *configv1alpha1.CredentialIssuerStatus) { - configToUpdate.KubeConfigInfo.CertificateAuthorityData = "initial-ca-value" - - t := configToUpdate.Strategies[0].LastUpdateTime - loc, err := time.LoadLocation("Asia/Shanghai") - r.NoError(err) - configToUpdate.Strategies[0].LastUpdateTime = metav1.NewTime(t.In(loc)) - }, - ) - r.NoError(err) - - expectedGetAction := coretesting.NewRootGetAction(credentialIssuerGVR, credentialIssuerResourceName) - r.Equal([]coretesting.Action{expectedGetAction}, pinnipedAPIClient.Actions()) - }) - - when("there is an unexpected error while getting the existing object", func() { - it.Before(func() { - pinnipedAPIClient.PrependReactor("get", "credentialissuers", func(_ coretesting.Action) (bool, runtime.Object, error) { - return true, nil, fmt.Errorf("error on get") - }) - }) - - it("returns an error", func() { - err := CreateOrUpdateCredentialIssuerStatus( - ctx, - credentialIssuerResourceName, - map[string]string{}, - pinnipedAPIClient, - func(configToUpdate *configv1alpha1.CredentialIssuerStatus) {}, - ) - r.EqualError(err, "could not create or update credentialissuer: get failed: error on get") - }) - }) - - when("there is an unexpected error while updating the existing object", func() { - it.Before(func() { - pinnipedAPIClient.PrependReactor("update", "credentialissuers", func(_ coretesting.Action) (bool, runtime.Object, error) { - return true, nil, fmt.Errorf("error on update") - }) - }) - - it("returns an error", func() { - err := CreateOrUpdateCredentialIssuerStatus( - ctx, - credentialIssuerResourceName, - map[string]string{}, - pinnipedAPIClient, - func(configToUpdate *configv1alpha1.CredentialIssuerStatus) { - configToUpdate.KubeConfigInfo.CertificateAuthorityData = "new-ca-value" - }, - ) - r.EqualError(err, "could not create or update credentialissuer: error on update") - }) - }) - - when("there is a conflict error while updating the existing object on the first try and the next try succeeds", func() { - var slightlyDifferentExistingConfig *configv1alpha1.CredentialIssuer - - it.Before(func() { - hit := false - slightlyDifferentExistingConfig = existingConfig.DeepCopy() - slightlyDifferentExistingConfig.Status.KubeConfigInfo.Server = "some-other-server-value-from-conflicting-update" - - pinnipedAPIClient.PrependReactor("update", "credentialissuers", func(_ coretesting.Action) (bool, runtime.Object, error) { - // Return an error on the first call, then fall through to the default (successful) response. - if !hit { - // Before the update fails, also change the object that will be returned by the next Get(), - // to make sure that the production code does a fresh Get() after detecting a conflict. - r.NoError(pinnipedAPIClient.Tracker().Update(credentialIssuerGVR, slightlyDifferentExistingConfig, "")) - hit = true - return true, nil, apierrors.NewConflict(schema.GroupResource{ - Group: apiregistrationv1.GroupName, - Resource: "credentialissuers", - }, "alphav1.pinniped.dev", fmt.Errorf("there was a conflict")) - } - return false, nil, nil - }) - }) - - it("retries updates on conflict", func() { - err := CreateOrUpdateCredentialIssuerStatus( - ctx, - credentialIssuerResourceName, - map[string]string{ - "myLabelKey1": "myLabelValue1", - "myLabelKey2": "myLabelValue2", - }, - pinnipedAPIClient, - func(configToUpdate *configv1alpha1.CredentialIssuerStatus) { - configToUpdate.KubeConfigInfo.CertificateAuthorityData = "new-ca-value" - }, - ) - r.NoError(err) - - expectedGetAction := coretesting.NewRootGetAction(credentialIssuerGVR, credentialIssuerResourceName) - - // The first attempted update only includes its own edits. - firstExpectedUpdatedConfig := existingConfig.DeepCopy() - firstExpectedUpdatedConfig.Status.KubeConfigInfo.CertificateAuthorityData = "new-ca-value" - 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.NewRootUpdateSubresourceAction(credentialIssuerGVR, "status", secondExpectedUpdatedConfig) - - expectedActions := []coretesting.Action{ - expectedGetAction, - firstExpectedUpdateAction, - expectedGetAction, - secondExpectedUpdateAction, - } - r.Equal(expectedActions, pinnipedAPIClient.Actions()) - }) - }) - }) - }, spec.Parallel(), spec.Report(report.Terminal{})) -} diff --git a/internal/controller/issuerconfig/doc.go b/internal/controller/issuerconfig/doc.go deleted file mode 100644 index a30f1283..00000000 --- a/internal/controller/issuerconfig/doc.go +++ /dev/null @@ -1,5 +0,0 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -// Package issuerconfig contains controller(s) for reconciling CredentialIssuer's. -package issuerconfig diff --git a/internal/controller/issuerconfig/update_strategy.go b/internal/controller/issuerconfig/issuerconfig.go similarity index 82% rename from internal/controller/issuerconfig/update_strategy.go rename to internal/controller/issuerconfig/issuerconfig.go index 5d079136..b2440203 100644 --- a/internal/controller/issuerconfig/update_strategy.go +++ b/internal/controller/issuerconfig/issuerconfig.go @@ -1,6 +1,7 @@ // Copyright 2021 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 +// Package issuerconfig contains helpers for updating CredentialIssuer status entries. package issuerconfig import ( @@ -15,23 +16,6 @@ import ( "go.pinniped.dev/generated/latest/client/concierge/clientset/versioned" ) -// UpdateStrategy creates or updates the desired strategy in the CredentialIssuer status.strategies field. -// The CredentialIssuer will be created if it does not already exist. -func UpdateStrategy(ctx context.Context, - name string, - credentialIssuerLabels map[string]string, - pinnipedAPIClient versioned.Interface, - strategy v1alpha1.CredentialIssuerStrategy, -) error { - return CreateOrUpdateCredentialIssuerStatus( - ctx, - name, - credentialIssuerLabels, - pinnipedAPIClient, - func(configToUpdate *v1alpha1.CredentialIssuerStatus) { mergeStrategy(configToUpdate, strategy) }, - ) -} - // Update a strategy on an existing CredentialIssuer, merging into any existing strategy entries. func Update(ctx context.Context, client versioned.Interface, issuer *v1alpha1.CredentialIssuer, strategy v1alpha1.CredentialIssuerStrategy) error { // Update the existing object to merge in the new strategy. diff --git a/internal/controller/issuerconfig/update_strategy_test.go b/internal/controller/issuerconfig/issuerconfig_test.go similarity index 100% rename from internal/controller/issuerconfig/update_strategy_test.go rename to internal/controller/issuerconfig/issuerconfig_test.go