Merge pull request #1878 from anik120/auth-catsrc

Add authentication for private index images

Author: openshift-merge-robot

pkg/controller/operators/catalog/operator_test.go

@@ -1424,7 +1424,7 @@ func toManifest(t *testing.T, obj runtime.Object) string {
 }
 
 func pod(s v1alpha1.CatalogSource) *corev1.Pod {
-	pod := reconciler.Pod(&s, "registry-server", s.Spec.Image, s.GetLabels(), 5, 10)
+	pod := reconciler.Pod(&s, "registry-server", s.Spec.Image, s.GetName(), s.GetLabels(), 5, 10)
 	ownerutil.AddOwner(pod, &s, false, false)
 	return pod
 }

pkg/controller/registry/reconciler/configmap.go

@@ -95,7 +95,7 @@ func (s *configMapCatalogSourceDecorator) Service() *v1.Service {
 }
 
 func (s *configMapCatalogSourceDecorator) Pod(image string) *v1.Pod {
-	pod := Pod(s.CatalogSource, "configmap-registry-server", image, s.Labels(), 5, 2)
+	pod := Pod(s.CatalogSource, "configmap-registry-server", image, "", s.Labels(), 5, 2)
 	pod.Spec.ServiceAccountName = s.GetName() + ConfigMapServerPostfix
 	pod.Spec.Containers[0].Command = []string{"configmap-server", "-c", s.Spec.ConfigMap, "-n", s.GetNamespace()}
 	ownerutil.AddOwner(pod, s.CatalogSource, false, false)

pkg/controller/registry/reconciler/configmap_test.go

@@ -204,7 +204,7 @@ func objectsForCatalogSource(catsrc *v1alpha1.CatalogSource) []runtime.Object {
 		if catsrc.Spec.Image != "" {
 			decorated := grpcCatalogSourceDecorator{catsrc}
 			objs = clientfake.AddSimpleGeneratedNames(
-				decorated.Pod(),
+				decorated.Pod(""),
 				decorated.Service(),
 			)
 		}

pkg/controller/registry/reconciler/grpc.go

@@ -9,6 +9,8 @@ import (
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
+	k8serror "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -86,8 +88,34 @@ func (s *grpcCatalogSourceDecorator) Service() *corev1.Service {
 	return svc
 }
 
-func (s *grpcCatalogSourceDecorator) Pod() *corev1.Pod {
-	pod := Pod(s.CatalogSource, "registry-server", s.Spec.Image, s.Labels(), 5, 10)
+func (s *grpcCatalogSourceDecorator) ServiceAccount() *corev1.ServiceAccount {
+	var secrets []corev1.LocalObjectReference
+	blockOwnerDeletion := true
+	isController := true
+	for _, secretName := range s.CatalogSource.Spec.Secrets {
+		secrets = append(secrets, corev1.LocalObjectReference{Name: secretName})
+	}
+	return &corev1.ServiceAccount{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      s.GetName(),
+			Namespace: s.GetNamespace(),
+			OwnerReferences: []metav1.OwnerReference{
+				{
+					Name:               s.GetName(),
+					Kind:               v1alpha1.CatalogSourceKind,
+					APIVersion:         v1alpha1.CatalogSourceCRDAPIVersion,
+					UID:                s.GetUID(),
+					Controller:         &isController,
+					BlockOwnerDeletion: &blockOwnerDeletion,
+				},
+			},
+		},
+		ImagePullSecrets: secrets,
+	}
+}
+
+func (s *grpcCatalogSourceDecorator) Pod(saName string) *corev1.Pod {
+	pod := Pod(s.CatalogSource, "registry-server", s.Spec.Image, saName, s.Labels(), 5, 10)
 	ownerutil.AddOwner(pod, s.CatalogSource, false, false)
 	return pod
 }
@@ -160,14 +188,18 @@ func (c *GrpcRegistryReconciler) EnsureRegistryServer(catalogSource *v1alpha1.Ca
 	overwritePod := overwrite || len(c.currentPodsWithCorrectImage(source)) == 0
 
 	//TODO: if any of these error out, we should write a status back (possibly set RegistryServiceStatus to nil so they get recreated)
-	if err := c.ensurePod(source, overwritePod); err != nil {
-		return errors.Wrapf(err, "error ensuring pod: %s", source.Pod().GetName())
+	sa, err := c.ensureSA(source)
+	if err != nil && !k8serror.IsAlreadyExists(err) {
+		return errors.Wrapf(err, "error ensuring service account: %s", source.GetName())
+	}
+	if err := c.ensurePod(source, sa.GetName(), overwritePod); err != nil {
+		return errors.Wrapf(err, "error ensuring pod: %s", source.Pod(sa.Name).GetName())
 	}
-	if err := c.ensureUpdatePod(source); err != nil {
+	if err := c.ensureUpdatePod(source, sa.Name); err != nil {
 		if _, ok := err.(UpdateNotReadyErr); ok {
 			return err
 		}
-		return errors.Wrapf(err, "error ensuring updated catalog source pod: %s", source.Pod().GetName())
+		return errors.Wrapf(err, "error ensuring updated catalog source pod: %s", source.Pod(sa.Name).GetName())
 	}
 	if err := c.ensureService(source, overwrite); err != nil {
 		return errors.Wrapf(err, "error ensuring service: %s", source.Service().GetName())
@@ -186,7 +218,7 @@ func (c *GrpcRegistryReconciler) EnsureRegistryServer(catalogSource *v1alpha1.Ca
 	return nil
 }
 
-func (c *GrpcRegistryReconciler) ensurePod(source grpcCatalogSourceDecorator, overwrite bool) error {
+func (c *GrpcRegistryReconciler) ensurePod(source grpcCatalogSourceDecorator, saName string, overwrite bool) error {
 	// currentLivePods refers to the currently live instances of the catalog source
 	currentLivePods := c.currentPods(source)
 	if len(currentLivePods) > 0 {
@@ -199,16 +231,16 @@ func (c *GrpcRegistryReconciler) ensurePod(source grpcCatalogSourceDecorator, ov
 			}
 		}
 	}
-	_, err := c.OpClient.KubernetesInterface().CoreV1().Pods(source.GetNamespace()).Create(context.TODO(), source.Pod(), metav1.CreateOptions{})
+	_, err := c.OpClient.KubernetesInterface().CoreV1().Pods(source.GetNamespace()).Create(context.TODO(), source.Pod(saName), metav1.CreateOptions{})
 	if err != nil {
-		return errors.Wrapf(err, "error creating new pod: %s", source.Pod().GetGenerateName())
+		return errors.Wrapf(err, "error creating new pod: %s", source.Pod(saName).GetGenerateName())
 	}
 
 	return nil
 }
 
 // ensureUpdatePod checks that for the same catalog source version the same container imageID is running
-func (c *GrpcRegistryReconciler) ensureUpdatePod(source grpcCatalogSourceDecorator) error {
+func (c *GrpcRegistryReconciler) ensureUpdatePod(source grpcCatalogSourceDecorator, saName string) error {
 	if !source.Poll() {
 		return nil
 	}
@@ -218,7 +250,7 @@ func (c *GrpcRegistryReconciler) ensureUpdatePod(source grpcCatalogSourceDecorat
 
 	if source.Update() && len(currentUpdatePods) == 0 {
 		logrus.WithField("CatalogSource", source.GetName()).Infof("catalog update required at %s", time.Now().String())
-		pod, err := c.createUpdatePod(source)
+		pod, err := c.createUpdatePod(source, saName)
 		if err != nil {
 			return errors.Wrapf(err, "creating update catalog source pod")
 		}
@@ -283,6 +315,14 @@ func (c *GrpcRegistryReconciler) ensureService(source grpcCatalogSourceDecorator
 	return err
 }
 
+func (c *GrpcRegistryReconciler) ensureSA(source grpcCatalogSourceDecorator) (*v1.ServiceAccount, error) {
+	sa := source.ServiceAccount()
+	if _, err := c.OpClient.CreateServiceAccount(sa); err != nil {
+		return sa, err
+	}
+	return sa, nil
+}
+
 // ServiceHashMatch will check the hash info in existing Service to ensure its
 // hash info matches the desired Service's hash.
 func ServiceHashMatch(existing, new *corev1.Service) bool {
@@ -317,14 +357,14 @@ func HashServiceSpec(spec corev1.ServiceSpec) string {
 }
 
 // createUpdatePod is an internal method that creates a pod using the latest catalog source.
-func (c *GrpcRegistryReconciler) createUpdatePod(source grpcCatalogSourceDecorator) (*corev1.Pod, error) {
+func (c *GrpcRegistryReconciler) createUpdatePod(source grpcCatalogSourceDecorator, saName string) (*corev1.Pod, error) {
 	// remove label from pod to ensure service does not accidentally route traffic to the pod
-	p := source.Pod()
+	p := source.Pod(saName)
 	p = swapLabels(p, "", source.Name)
 
 	pod, err := c.OpClient.KubernetesInterface().CoreV1().Pods(source.GetNamespace()).Create(context.TODO(), p, metav1.CreateOptions{})
 	if err != nil {
-		logrus.WithField("pod", source.Pod().GetName()).Warn("couldn't create new catalogsource pod")
+		logrus.WithField("pod", source.Pod(saName).GetName()).Warn("couldn't create new catalogsource pod")
 		return nil, err
 	}
 

pkg/controller/registry/reconciler/grpc_test.go

@@ -33,8 +33,27 @@ func validGrpcCatalogSource(image, address string) *v1alpha1.CatalogSource {
 	}
 }
 
+func grpcCatalogSourceWithSecret(secretName string) *v1alpha1.CatalogSource {
+	return &v1alpha1.CatalogSource{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "private-catalog",
+			Namespace: testNamespace,
+			UID:       types.UID("catalog-uid"),
+			Labels:    map[string]string{"olm.catalogSource": "img-catalog"},
+		},
+		Spec: v1alpha1.CatalogSourceSpec{
+			Image:      "private-image",
+			Address:    "",
+			SourceType: v1alpha1.SourceTypeGrpc,
+			Secrets:    []string{secretName},
+		},
+	}
+}
+
 func TestGrpcRegistryReconciler(t *testing.T) {
 	now := func() metav1.Time { return metav1.Date(2018, time.January, 26, 20, 40, 0, 0, time.UTC) }
+	blockOwnerDeletion := true
+	isController := true
 
 	type cluster struct {
 		k8sObjs []runtime.Object
@@ -44,8 +63,9 @@ func TestGrpcRegistryReconciler(t *testing.T) {
 		catsrc  *v1alpha1.CatalogSource
 	}
 	type out struct {
-		status *v1alpha1.RegistryServiceStatus
-		err    error
+		status  *v1alpha1.RegistryServiceStatus
+		cluster cluster
+		err     error
 	}
 	tests := []struct {
 		testName string
@@ -186,6 +206,56 @@ func TestGrpcRegistryReconciler(t *testing.T) {
 				},
 			},
 		},
+		{
+			testName: "Grpc/PrivateRegistry/SAHasSecrets",
+			in: in{
+				cluster: cluster{
+					k8sObjs: []runtime.Object{
+						&corev1.Secret{
+							ObjectMeta: metav1.ObjectMeta{
+								Name:      "test-secret",
+								Namespace: testNamespace,
+							},
+						},
+					},
+				},
+				catsrc: grpcCatalogSourceWithSecret("test-secret"),
+			},
+			out: out{
+				status: &v1alpha1.RegistryServiceStatus{
+					CreatedAt:        now(),
+					Protocol:         "grpc",
+					ServiceName:      "private-catalog",
+					ServiceNamespace: testNamespace,
+					Port:             "50051",
+				},
+				cluster: cluster{
+					k8sObjs: []runtime.Object{
+						&corev1.ServiceAccount{
+							ObjectMeta: metav1.ObjectMeta{
+								Name:      "private-catalog",
+								Namespace: testNamespace,
+								OwnerReferences: []metav1.OwnerReference{
+									{
+										Name:               "private-catalog",
+										UID:                types.UID("catalog-uid"),
+										Kind:               v1alpha1.CatalogSourceKind,
+										APIVersion:         v1alpha1.CatalogSourceCRDAPIVersion,
+										BlockOwnerDeletion: &blockOwnerDeletion,
+										Controller:         &isController,
+									},
+								},
+							},
+							ImagePullSecrets: []corev1.LocalObjectReference{
+								{
+									Name: "test-secret",
+								},
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.testName, func(t *testing.T) {
@@ -206,12 +276,13 @@ func TestGrpcRegistryReconciler(t *testing.T) {
 
 			// Check for resource existence
 			decorated := grpcCatalogSourceDecorator{tt.in.catsrc}
-			pod := decorated.Pod()
+			pod := decorated.Pod(tt.in.catsrc.GetName())
 			service := decorated.Service()
+			sa := decorated.ServiceAccount()
 			listOptions := metav1.ListOptions{LabelSelector: labels.SelectorFromSet(labels.Set{CatalogSourceLabelKey: tt.in.catsrc.GetName()}).String()}
 			outPods, podErr := client.KubernetesInterface().CoreV1().Pods(pod.GetNamespace()).List(context.TODO(), listOptions)
 			outService, serviceErr := client.KubernetesInterface().CoreV1().Services(service.GetNamespace()).Get(context.TODO(), service.GetName(), metav1.GetOptions{})
-
+			outsa, saerr := client.KubernetesInterface().CoreV1().ServiceAccounts(sa.GetNamespace()).Get(context.TODO(), sa.GetName(), metav1.GetOptions{})
 			switch rec.(type) {
 			case *GrpcRegistryReconciler:
 				// Should be created by a GrpcRegistryReconciler
@@ -223,6 +294,10 @@ func TestGrpcRegistryReconciler(t *testing.T) {
 				require.Equal(t, pod.Spec, outPod.Spec)
 				require.NoError(t, serviceErr)
 				require.Equal(t, service, outService)
+				require.NoError(t, saerr)
+				if len(tt.in.catsrc.Spec.Secrets) > 0 {
+					require.Equal(t, tt.out.cluster.k8sObjs[0], outsa)
+				}
 			case *GrpcAddressRegistryReconciler:
 				// Should not be created by a GrpcAddressRegistryReconciler
 				require.NoError(t, podErr)

pkg/controller/registry/reconciler/reconciler.go

@@ -91,7 +91,7 @@ func NewRegistryReconcilerFactory(lister operatorlister.OperatorLister, opClient
 	}
 }
 
-func Pod(source *v1alpha1.CatalogSource, name string, image string, labels map[string]string, readinessDelay int32, livenessDelay int32) *v1.Pod {
+func Pod(source *v1alpha1.CatalogSource, name string, image string, saName string, labels map[string]string, readinessDelay int32, livenessDelay int32) *v1.Pod {
 	// Ensure the catalog image is always pulled if the image is not based on a digest, measured by whether an "@" is included.
 	// See https://github.com/docker/distribution/blob/master/reference/reference.go for more info.
 	// This means recreating non-digest based catalog pods will result in the latest version of the catalog content being delivered on-cluster.
@@ -148,6 +148,7 @@ func Pod(source *v1alpha1.CatalogSource, name string, image string, labels map[s
 			NodeSelector: map[string]string{
 				"kubernetes.io/os": "linux",
 			},
+			ServiceAccountName: saName,
 		},
 	}
 	return pod

pkg/controller/registry/reconciler/reconciler_test.go

@@ -19,7 +19,7 @@ func TestPodNodeSelector(t *testing.T) {
 	key := "kubernetes.io/os"
 	value := "linux"
 
-	gotCatSrcPod := Pod(catsrc, "hello", "busybox", map[string]string{}, int32(0), int32(0))
+	gotCatSrcPod := Pod(catsrc, "hello", "busybox", "", map[string]string{}, int32(0), int32(0))
 	gotCatSrcPodSelector := gotCatSrcPod.Spec.NodeSelector
 
 	if gotCatSrcPodSelector[key] != value {
@@ -67,7 +67,7 @@ func TestPullPolicy(t *testing.T) {
 	}
 
 	for _, tt := range table {
-		p := Pod(source, "catalog", tt.image, nil, int32(0), int32(0))
+		p := Pod(source, "catalog", tt.image, "", nil, int32(0), int32(0))
 		policy := p.Spec.Containers[0].ImagePullPolicy
 		if policy != tt.policy {
 			t.Fatalf("expected pull policy %s for image  %s", tt.policy, tt.image)