Fix apache#1799: added system:image-puller delegation for OpenShift

deploy/traits.yaml

@@ -533,6 +533,9 @@ traits:
   - name: secret-name
     type: string
     description: The pull secret name to set on the Pod. If left empty this is automatically taken from the `IntegrationPlatform` registry configuration.
+  - name: image-puller-delegation
+    type: bool
+    description: When using a global operator with a shared platform, this enables delegation of the `system:image-puller` cluster role on the operator namespace to the integration service account.
   - name: auto
     type: bool
     description: Automatically configures the platform registry secret on the pod if it is of type `kubernetes.io/dockerconfigjson`.

docs/modules/ROOT/assets/attachments/schema/integration-schema.json

@@ -433,6 +433,9 @@
         "platform": {
           "type": "string"
         },
+        "platformNamespace": {
+          "type": "string"
+        },
         "profile": {
           "description": "TraitProfile represents lists of traits that are enabled for the specific installation/integration",
           "type": "string"

docs/modules/traits/pages/pull-secret.adoc

@@ -38,6 +38,10 @@ The following configuration options are available:
 | string
 | The pull secret name to set on the Pod. If left empty this is automatically taken from the `IntegrationPlatform` registry configuration.
 
+| pull-secret.image-puller-delegation
+| bool
+| When using a global operator with a shared platform, this enables delegation of the `system:image-puller` cluster role on the operator namespace to the integration service account.
+
 | pull-secret.auto
 | bool
 | Automatically configures the platform registry secret on the pod if it is of type `kubernetes.io/dockerconfigjson`.

pkg/controller/integration/integration_controller.go

@@ -150,20 +150,26 @@ func add(mgr manager.Manager, r reconcile.Reconciler, c client.Client) error {
 	// requests for any integrations that are in phase waiting for platform
 	err = ctrl.Watch(&source.Kind{Type: &v1.IntegrationPlatform{}}, &handler.EnqueueRequestsFromMapFunc{
 		ToRequests: handler.ToRequestsFunc(func(a handler.MapObject) []reconcile.Request {
-			platform := a.Object.(*v1.IntegrationPlatform)
+			pl := a.Object.(*v1.IntegrationPlatform)
 			var requests []reconcile.Request
 
-			if platform.Status.Phase == v1.IntegrationPlatformPhaseReady {
+			if pl.Status.Phase == v1.IntegrationPlatformPhaseReady {
 				list := &v1.IntegrationList{}
 
-				if err := mgr.GetClient().List(context.TODO(), list, k8sclient.InNamespace(platform.Namespace)); err != nil {
+				// Do global search in case of global operator (it may be using a global platform)
+				var opts []k8sclient.ListOption
+				if !platform.IsCurrentOperatorGlobal() {
+					opts = append(opts, k8sclient.InNamespace(pl.Namespace))
+				}
+
+				if err := mgr.GetClient().List(context.TODO(), list, opts...); err != nil {
 					log.Error(err, "Failed to retrieve integration list")
 					return requests
 				}
 
 				for _, integration := range list.Items {
 					if integration.Status.Phase == v1.IntegrationPhaseWaitingForPlatform {
-						log.Infof("Platform %s ready, wake-up integration: %s", platform.Name, integration.Name)
+						log.Infof("Platform %s ready, wake-up integration: %s", pl.Name, integration.Name)
 						requests = append(requests, reconcile.Request{
 							NamespacedName: types.NamespacedName{
 								Namespace: integration.Namespace,

pkg/platform/defaults.go

@@ -76,14 +76,23 @@ func ConfigureDefaults(ctx context.Context, c client.Client, p *v1.IntegrationPl
 	}
 
 	if p.Status.Build.BuildStrategy == "" {
-		// If the operator is global, a global build strategy should be used
-		if IsCurrentOperatorGlobal() {
+		operatorIsLocal := !IsCurrentOperatorGlobal()
+		// Local operators build in the same namespace by definition
+		buildInDifferentNamespace := !operatorIsLocal
+		if !operatorIsLocal {
+			// Unless it's a global operator using a globally shared integration platform
+			buildInDifferentNamespace = p.Namespace != GetOperatorNamespace()
+		}
+
+		// If the operator and the build are in different namespaces, pod strategy should be used (except for Spectrum)
+		if buildInDifferentNamespace {
 			if p.Status.Build.PublishStrategy == v1.IntegrationPlatformBuildPublishStrategySpectrum {
 				p.Status.Build.BuildStrategy = v1.IntegrationPlatformBuildStrategyRoutine
 			} else {
 				p.Status.Build.BuildStrategy = v1.IntegrationPlatformBuildStrategyPod
 			}
 		} else {
+			// Same-namespace builds use the fastest strategy that they support (routine when possible)
 			if p.Status.Build.PublishStrategy == v1.IntegrationPlatformBuildPublishStrategyS2I ||
 				p.Status.Build.PublishStrategy == v1.IntegrationPlatformBuildPublishStrategySpectrum {
 				p.Status.Build.BuildStrategy = v1.IntegrationPlatformBuildStrategyRoutine

pkg/trait/cron_test.go

@@ -365,80 +365,6 @@ func TestCronDepsFallback(t *testing.T) {
 	assert.Contains(t, environment.Integration.Status.Dependencies, "mvn:org.apache.camel.k:camel-k-cron")
 }
 
-func TestCronWithMain(t *testing.T) {
-	catalog, err := camel.DefaultCatalog()
-	assert.Nil(t, err)
-
-	traitCatalog := NewCatalog(context.TODO(), nil)
-
-	environment := Environment{
-		CamelCatalog: catalog,
-		Catalog:      traitCatalog,
-		Integration: &v1.Integration{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "test",
-				Namespace: "ns",
-			},
-			Status: v1.IntegrationStatus{
-				Phase: v1.IntegrationPhaseDeploying,
-			},
-			Spec: v1.IntegrationSpec{
-				Profile: v1.TraitProfileKnative,
-				Sources: []v1.SourceSpec{
-					{
-						DataSpec: v1.DataSpec{
-							Name:    "routes.java",
-							Content: `from("cron:tab?schedule=0 0/2 * * ?").to("log:test")`,
-						},
-						Language: v1.LanguageJavaSource,
-					},
-				},
-				Resources: []v1.ResourceSpec{},
-				Traits: map[string]v1.TraitSpec{
-					"quarkus": test.TraitSpecFromMap(t, map[string]interface{}{
-						"enabled": false,
-					}),
-				},
-			},
-		},
-		IntegrationKit: &v1.IntegrationKit{
-			Status: v1.IntegrationKitStatus{
-				Phase: v1.IntegrationKitPhaseReady,
-			},
-		},
-		Platform: &v1.IntegrationPlatform{
-			Spec: v1.IntegrationPlatformSpec{
-				Cluster: v1.IntegrationPlatformClusterOpenShift,
-				Build: v1.IntegrationPlatformBuildSpec{
-					PublishStrategy: v1.IntegrationPlatformBuildPublishStrategyS2I,
-					Registry:        v1.IntegrationPlatformRegistrySpec{Address: "registry"},
-				},
-				Profile: v1.TraitProfileKnative,
-			},
-		},
-		EnvVars:        make([]corev1.EnvVar, 0),
-		ExecutedTraits: make([]Trait, 0),
-		Resources:      k8sutils.NewCollection(),
-	}
-	environment.Platform.ResyncStatusFullConfig()
-
-	c, err := NewFakeClient("ns")
-	assert.Nil(t, err)
-
-	tc := NewCatalog(context.TODO(), c)
-
-	err = tc.apply(&environment)
-
-	assert.Nil(t, err)
-	assert.NotEmpty(t, environment.ExecutedTraits)
-	assert.Nil(t, environment.GetTrait("quarkus"))
-
-	ct := environment.GetTrait("cron").(*cronTrait)
-	assert.NotNil(t, ct)
-	assert.Nil(t, ct.Fallback)
-	assert.Contains(t, environment.Interceptors, "cron")
-}
-
 func TestCronWithQuarkus(t *testing.T) {
 	catalog, err := camel.DefaultCatalog()
 	assert.Nil(t, err)

pkg/trait/pull_secret.go

@@ -18,9 +18,17 @@ limitations under the License.
 package trait
 
 import (
+	"fmt"
+
 	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"github.com/apache/camel-k/pkg/platform"
 	"github.com/apache/camel-k/pkg/util"
+	"github.com/apache/camel-k/pkg/util/kubernetes"
+	"github.com/apache/camel-k/pkg/util/openshift"
+	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -40,6 +48,8 @@ type pullSecretTrait struct {
 	BaseTrait `property:",squash"`
 	// The pull secret name to set on the Pod. If left empty this is automatically taken from the `IntegrationPlatform` registry configuration.
 	SecretName string `property:"secret-name" json:"secretName,omitempty"`
+	// When using a global operator with a shared platform, this enables delegation of the `system:image-puller` cluster role on the operator namespace to the integration service account.
+	ImagePullerDelegation *bool `property:"image-puller-delegation" json:"imagePullerDelegation,omitempty"`
 	// Automatically configures the platform registry secret on the pod if it is of type `kubernetes.io/dockerconfigjson`.
 	Auto *bool `property:"auto" json:"auto,omitempty"`
 }
@@ -73,17 +83,72 @@ func (t *pullSecretTrait) Configure(e *Environment) (bool, error) {
 				}
 			}
 		}
+		if t.ImagePullerDelegation == nil {
+			var isOpenshift bool
+			if t.Client != nil {
+				var err error
+				isOpenshift, err = openshift.IsOpenShift(t.Client)
+				if err != nil {
+					return false, err
+				}
+			}
+			isOperatorGlobal := platform.IsCurrentOperatorGlobal()
+			isKitExternal := e.Integration.GetIntegrationKitNamespace() != e.Integration.Namespace
+			needsDelegation := isOpenshift && isOperatorGlobal && isKitExternal
+			t.ImagePullerDelegation = &needsDelegation
+		}
 	}
 
-	return t.SecretName != "", nil
+	return t.SecretName != "" || util.IsTrue(t.ImagePullerDelegation), nil
 }
 
 func (t *pullSecretTrait) Apply(e *Environment) error {
-	e.Resources.VisitPodSpec(func(p *corev1.PodSpec) {
-		p.ImagePullSecrets = append(p.ImagePullSecrets, corev1.LocalObjectReference{
-			Name: t.SecretName,
+	if t.SecretName != "" {
+		e.Resources.VisitPodSpec(func(p *corev1.PodSpec) {
+			p.ImagePullSecrets = append(p.ImagePullSecrets, corev1.LocalObjectReference{
+				Name: t.SecretName,
+			})
 		})
-	})
+	}
+	if util.IsTrue(t.ImagePullerDelegation) {
+		if err := t.delegateImagePuller(e); err != nil {
+			return err
+		}
+	}
 
 	return nil
 }
+
+func (t *pullSecretTrait) delegateImagePuller(e *Environment) error {
+	// Applying the rolebinding directly because it's a resource in the operator namespace
+	// (different from the integration namespace when delegation is enabled).
+	rb := t.newImagePullerRoleBinding(e)
+	if err := kubernetes.ReplaceResource(e.C, e.Client, rb); err != nil {
+		return errors.Wrap(err, "error during the creation of the system:image-puller delegating role binding")
+	}
+	return nil
+}
+
+func (t *pullSecretTrait) newImagePullerRoleBinding(e *Environment) *rbacv1.RoleBinding {
+	serviceAccount := e.Integration.Spec.ServiceAccountName
+	if serviceAccount == "" {
+		serviceAccount = "default"
+	}
+	return &rbacv1.RoleBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: e.Integration.GetIntegrationKitNamespace(),
+			Name:      fmt.Sprintf("camel-k-puller-%s", e.Integration.Namespace),
+		},
+		RoleRef: rbacv1.RoleRef{
+			Kind: "ClusterRole",
+			Name: "system:image-puller",
+		},
+		Subjects: []rbacv1.Subject{
+			{
+				Kind:      "ServiceAccount",
+				Namespace: e.Integration.Namespace,
+				Name:      serviceAccount,
+			},
+		},
+	}
+}

pkg/util/test/client.go

@@ -26,8 +26,11 @@ import (
 	fakecamelclientset "github.com/apache/camel-k/pkg/client/camel/clientset/versioned/fake"
 	camelv1 "github.com/apache/camel-k/pkg/client/camel/clientset/versioned/typed/camel/v1"
 	camelv1alpha1 "github.com/apache/camel-k/pkg/client/camel/clientset/versioned/typed/camel/v1alpha1"
+	k8serrors "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"
+	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/kubernetes"
 	fakeclientset "k8s.io/client-go/kubernetes/fake"
 	clientscheme "k8s.io/client-go/kubernetes/scheme"
@@ -101,3 +104,23 @@ func (c *FakeClient) GetConfig() *rest.Config {
 func (c *FakeClient) GetCurrentNamespace(kubeConfig string) (string, error) {
 	return "", nil
 }
+
+func (c *FakeClient) Discovery() discovery.DiscoveryInterface {
+	return &FakeDiscovery{
+		DiscoveryInterface: c.Interface.Discovery(),
+	}
+}
+
+type FakeDiscovery struct {
+	discovery.DiscoveryInterface
+}
+
+func (f *FakeDiscovery) ServerResourcesForGroupVersion(groupVersion string) (*metav1.APIResourceList, error) {
+	// Normalize the fake discovery to behave like the real implementation when checking for openshift
+	if groupVersion == "image.openshift.io/v1" {
+		return nil, k8serrors.NewNotFound(schema.GroupResource{
+			Group: "image.openshift.io",
+		}, "")
+	}
+	return f.DiscoveryInterface.ServerResourcesForGroupVersion(groupVersion)
+}