diff --git a/pkg/controller/operators/catalog/operator_test.go b/pkg/controller/operators/catalog/operator_test.go index 9283a4b460..ca4c324aad 100644 --- a/pkg/controller/operators/catalog/operator_test.go +++ b/pkg/controller/operators/catalog/operator_test.go @@ -382,6 +382,96 @@ func TestExecutePlan(t *testing.T) { want: []runtime.Object{service("service", namespace), csv("csv", namespace, nil, nil)}, err: nil, }, + { + testName: "CreateServiceAccount", + in: withSteps(installPlan("p", namespace, v1alpha1.InstallPlanPhaseInstalling, "csv"), + []*v1alpha1.Step{ + { + Resource: v1alpha1.StepResource{ + CatalogSource: "catalog", + CatalogSourceNamespace: namespace, + Group: "", + Version: "v1", + Kind: "ServiceAccount", + Name: "sa", + Manifest: toManifest(serviceAccount("sa", namespace, "", + objectReference("init secret"))), + }, + Status: v1alpha1.StepStatusUnknown, + }, + }, + ), + want: []runtime.Object{serviceAccount("sa", namespace, "", objectReference("init secret"))}, + err: nil, + }, + { + testName: "UpdateServiceAccountWithSameFields", + in: withSteps(installPlan("p", namespace, v1alpha1.InstallPlanPhaseInstalling, "csv"), + []*v1alpha1.Step{ + { + Resource: v1alpha1.StepResource{ + CatalogSource: "catalog", + CatalogSourceNamespace: namespace, + Group: "", + Version: "v1", + Kind: "ServiceAccount", + Name: "sa", + Manifest: toManifest(serviceAccount("sa", namespace, "name", + objectReference("init secret"))), + }, + Status: v1alpha1.StepStatusUnknown, + }, + { + Resource: v1alpha1.StepResource{ + CatalogSource: "catalog", + CatalogSourceNamespace: namespace, + Group: "", + Version: "v1", + Kind: "ServiceAccount", + Name: "sa", + Manifest: toManifest(serviceAccount("sa", namespace, "name", nil)), + }, + Status: v1alpha1.StepStatusUnknown, + }, + }, + ), + want: []runtime.Object{serviceAccount("sa", namespace, "name", objectReference("init secret"))}, + err: nil, + }, + { + testName: "UpdateServiceAccountWithDiffFields", + in: withSteps(installPlan("p", namespace, v1alpha1.InstallPlanPhaseInstalling, "csv"), + []*v1alpha1.Step{ + { + Resource: v1alpha1.StepResource{ + CatalogSource: "catalog", + CatalogSourceNamespace: namespace, + Group: "", + Version: "v1", + Kind: "ServiceAccount", + Name: "sa", + Manifest: toManifest(serviceAccount("sa", namespace, "old_name", + objectReference("init secret"))), + }, + Status: v1alpha1.StepStatusUnknown, + }, + { + Resource: v1alpha1.StepResource{ + CatalogSource: "catalog", + CatalogSourceNamespace: namespace, + Group: "", + Version: "v1", + Kind: "ServiceAccount", + Name: "sa", + Manifest: toManifest(serviceAccount("sa", namespace, "new_name", nil)), + }, + Status: v1alpha1.StepStatusUnknown, + }, + }, + ), + want: []runtime.Object{serviceAccount("sa", namespace, "new_name", objectReference("init secret"))}, + err: nil, + }, } for _, tt := range tests { @@ -411,6 +501,8 @@ func TestExecutePlan(t *testing.T) { fetched, err = op.opClient.GetRoleBinding(namespace, o.GetName()) case *corev1.ServiceAccount: fetched, err = op.opClient.GetServiceAccount(namespace, o.GetName()) + case *corev1.Secret: + fetched, err = op.opClient.GetSecret(namespace, o.GetName()) case *corev1.Service: fetched, err = op.opClient.GetService(namespace, o.GetName()) case *v1alpha1.ClusterServiceVersion: @@ -1010,6 +1102,25 @@ func service(name, namespace string) *corev1.Service { } } +func serviceAccount(name, namespace, generateName string, secretRef *corev1.ObjectReference) *corev1.ServiceAccount { + if secretRef == nil { + return &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace, GenerateName: generateName}, + } + } + return &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace, GenerateName: generateName}, + Secrets: []corev1.ObjectReference{*secretRef}, + } +} + +func objectReference(name string) *corev1.ObjectReference { + if name == "" { + return &corev1.ObjectReference{} + } + return &corev1.ObjectReference{Name: name} +} + func toManifest(obj runtime.Object) string { raw, _ := json.Marshal(obj) return string(raw) diff --git a/pkg/controller/operators/catalog/step_ensurer.go b/pkg/controller/operators/catalog/step_ensurer.go index eeccb9a42a..32fae51826 100644 --- a/pkg/controller/operators/catalog/step_ensurer.go +++ b/pkg/controller/operators/catalog/step_ensurer.go @@ -3,15 +3,17 @@ package catalog import ( "fmt" - "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1" - "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned" - "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient" - "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil" errorwrap "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" + apiequality "k8s.io/apimachinery/pkg/api/equality" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1" + "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned" + "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient" + "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil" ) func newStepEnsurer(kubeClient operatorclient.ClientInterface, crClient versioned.Interface) *StepEnsurer { @@ -116,11 +118,24 @@ func (o *StepEnsurer) EnsureServiceAccount(namespace string, sa *corev1.ServiceA return } - sa.SetNamespace(namespace) - if _, updateErr := o.kubeClient.UpdateServiceAccount(sa); updateErr != nil { - err = errorwrap.Wrapf(updateErr, "error updating service account: %s", sa.GetName()) + // Carrying secrets through the service account update. + preSa, getErr := o.kubeClient.KubernetesInterface().CoreV1().ServiceAccounts(namespace).Get(sa.Name, + metav1.GetOptions{}) + if getErr != nil { + err = errorwrap.Wrapf(getErr, "error getting older version of service account: %s", sa.GetName()) return } + sa.Secrets = preSa.Secrets + + sa.SetNamespace(namespace) + + // Use DeepDerivative to check if new SA is the same as the old SA. If no field is changed, we skip the update call. + if !apiequality.Semantic.DeepDerivative(sa, preSa) { + if _, updateErr := o.kubeClient.UpdateServiceAccount(sa); updateErr != nil { + err = errorwrap.Wrapf(updateErr, "error updating service account: %s", sa.GetName()) + return + } + } status = v1alpha1.StepStatusPresent return diff --git a/test/e2e/installplan_e2e_test.go b/test/e2e/installplan_e2e_test.go index fa6940b79e..f907b84927 100644 --- a/test/e2e/installplan_e2e_test.go +++ b/test/e2e/installplan_e2e_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/blang/semver" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" appsv1 "k8s.io/api/apps/v1" authorizationv1 "k8s.io/api/authorization/v1" @@ -1610,6 +1611,9 @@ func TestUpdateCatalogForSubscription(t *testing.T) { }, } + oldSecrets, err := c.KubernetesInterface().CoreV1().Secrets(testNamespace).List(metav1.ListOptions{}) + require.NoError(t, err, "error listing secrets") + // Create the catalog sources updatedNamedStrategy := newNginxInstallStrategy(genName("dep-"), updatedPermissions, updatedClusterPermissions) updatedCSV := newCSV(mainPackageStable+"-next", testNamespace, mainCSV.GetName(), semver.MustParse("0.2.0"), []apiextensions.CustomResourceDefinition{mainCRD}, nil, updatedNamedStrategy) @@ -1640,6 +1644,13 @@ func TestUpdateCatalogForSubscription(t *testing.T) { _, err = awaitCSV(t, crc, testNamespace, updatedCSV.GetName(), csvSucceededChecker) require.NoError(t, err) + newSecrets, err := c.KubernetesInterface().CoreV1().Secrets(testNamespace).List(metav1.ListOptions{}) + require.NoError(t, err, "error listing secrets") + // Assert that the number of secrets is not increased from updating service account as part of the install plan, + assert.EqualValues(t, len(oldSecrets.Items), len(newSecrets.Items)) + // And that the secret list is indeed updated. + assert.Equal(t, oldSecrets.Items, newSecrets.Items) + // Wait for ServiceAccount to not have access anymore err = wait.Poll(pollInterval, pollDuration, func() (bool, error) { res, err := c.KubernetesInterface().AuthorizationV1().SubjectAccessReviews().Create(&authorizationv1.SubjectAccessReview{