Repository: apache/camel-k
Pull request #2546 (2021-08-04 14:21:57)

Backport #2535 to 1.5.x
Back port #2535 to 1.5.x as automated back port failed because of conflicts.

**Release Note**
```release-note
Fix ImagePullError when using global operator in OpenShift
```



Patch:
diff --git a/pkg/trait/owner.go b/pkg/trait/owner.go
index c821e0264..a95b67a99 100644
--- a/pkg/trait/owner.go
+++ b/pkg/trait/owner.go
@@ -78,17 +78,22 @@ func (t *ownerTrait) Apply(e *Environment) error {
 	}
 
 	e.Resources.VisitMetaObject(func(res metav1.Object) {
-		references := []metav1.OwnerReference{
-			{
-				APIVersion:         e.Integration.APIVersion,
-				Kind:               e.Integration.Kind,
-				Name:               e.Integration.Name,
-				UID:                e.Integration.UID,
-				Controller:         &controller,
-				BlockOwnerDeletion: &blockOwnerDeletion,
-			},
+		// Cross-namespace references are forbidden and also asynchronously refused
+		// by the api server (sometimes no error is thrown but the resource is not created).
+		// Ref: https://github.com/kubernetes/kubernetes/issues/65200
+		if res.GetNamespace() == "" || res.GetNamespace() == e.Integration.Namespace {
+			references := []metav1.OwnerReference{
+				{
+					APIVersion:         e.Integration.APIVersion,
+					Kind:               e.Integration.Kind,
+					Name:               e.Integration.Name,
+					UID:                e.Integration.UID,
+					Controller:         &controller,
+					BlockOwnerDeletion: &blockOwnerDeletion,
+				},
+			}
+			res.SetOwnerReferences(references)
 		}
-		res.SetOwnerReferences(references)
 
 		// Transfer annotations
 		t.propagateLabelAndAnnotations(res, targetLabels, targetAnnotations)
diff --git a/pkg/trait/pull_secret.go b/pkg/trait/pull_secret.go
index 115d60142..82b3c64f7 100644
--- a/pkg/trait/pull_secret.go
+++ b/pkg/trait/pull_secret.go
@@ -23,7 +23,9 @@ import (
 	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"
@@ -109,22 +111,54 @@ func (t *pullSecretTrait) Apply(e *Environment) error {
 		})
 	}
 	if util.IsTrue(t.ImagePullerDelegation) {
-		rb := t.newImagePullerRoleBinding(e)
-		e.Resources.Add(rb)
+		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 {
+	targetNamespace := e.Integration.GetIntegrationKitNamespace(e.Platform)
+	var references []metav1.OwnerReference
+	if e.Platform != nil && e.Platform.Namespace == targetNamespace {
+		controller := true
+		blockOwnerDeletion := true
+		references = []metav1.OwnerReference{
+			{
+				APIVersion:         e.Platform.APIVersion,
+				Kind:               e.Platform.Kind,
+				Name:               e.Platform.Name,
+				UID:                e.Platform.UID,
+				Controller:         &controller,
+				BlockOwnerDeletion: &blockOwnerDeletion,
+			},
+		}
+	}
 	serviceAccount := e.Integration.Spec.ServiceAccountName
 	if serviceAccount == "" {
 		serviceAccount = "default"
 	}
 	return &rbacv1.RoleBinding{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "RoleBinding",
+			APIVersion: rbacv1.SchemeGroupVersion.String(),
+		},
 		ObjectMeta: metav1.ObjectMeta{
-			Namespace: e.Integration.GetIntegrationKitNamespace(e.Platform),
-			Name:      fmt.Sprintf("camel-k-puller-%s", e.Integration.Namespace),
+			Namespace:       targetNamespace,
+			Name:            fmt.Sprintf("camel-k-puller-%s-%s", e.Integration.Namespace, serviceAccount),
+			OwnerReferences: references,
 		},
 		RoleRef: rbacv1.RoleRef{
 			Kind: "ClusterRole",


Test patch:
diff --git a/pkg/trait/pull_secret_test.go b/pkg/trait/pull_secret_test.go
index a1499e014..44ee3efa4 100644
--- a/pkg/trait/pull_secret_test.go
+++ b/pkg/trait/pull_secret_test.go
@@ -18,32 +18,23 @@ limitations under the License.
 package trait
 
 import (
+	"context"
 	"testing"
 
 	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
 	"github.com/apache/camel-k/pkg/util/kubernetes"
+	"github.com/apache/camel-k/pkg/util/test"
 	appsv1 "k8s.io/api/apps/v1"
 	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"
 
 	"github.com/stretchr/testify/assert"
 )
 
 func TestPullSecret(t *testing.T) {
-	e := &Environment{}
-	e.Integration = &v1.Integration{
-		Status: v1.IntegrationStatus{
-			Phase: v1.IntegrationPhaseDeploying,
-		},
-	}
-
-	deployment := appsv1.Deployment{
-		Spec: appsv1.DeploymentSpec{
-			Template: corev1.PodTemplateSpec{
-				Spec: corev1.PodSpec{},
-			},
-		},
-	}
-	e.Resources = kubernetes.NewCollection(&deployment)
+	e, deployment := getEnvironmentAndDeployment(t)
 
 	trait := newPullSecretTrait().(*pullSecretTrait)
 	trait.SecretName = "xxxy"
@@ -57,15 +48,66 @@ func TestPullSecret(t *testing.T) {
 }
 
 func TestPullSecretDoesNothingWhenNotSetOnPlatform(t *testing.T) {
+	e, _ := getEnvironmentAndDeployment(t)
+	e.Platform = &v1.IntegrationPlatform{}
+
+	trait := newPullSecretTrait()
+	enabled, err := trait.Configure(e)
+	assert.Nil(t, err)
+	assert.False(t, enabled)
+}
+
+func TestPullSecretAuto(t *testing.T) {
+	e, _ := getEnvironmentAndDeployment(t)
+
+	trait := newPullSecretTrait().(*pullSecretTrait)
+	trait.Auto = newFalse()
+	enabled, err := trait.Configure(e)
+	assert.Nil(t, err)
+	assert.False(t, enabled)
+}
+
+func TestPullSecretImagePullerDelegation(t *testing.T) {
+	e, _ := getEnvironmentAndDeployment(t)
+
+	trait := newPullSecretTrait().(*pullSecretTrait)
+	trait.Auto = newFalse()
+	trait.ImagePullerDelegation = newTrue()
+	enabled, err := trait.Configure(e)
+	assert.Nil(t, err)
+	assert.True(t, enabled)
+	assert.True(t, *trait.ImagePullerDelegation)
+
+	err = trait.Apply(e)
+	assert.NoError(t, err)
+
+	var roleBinding rbacv1.RoleBinding
+	roleBindingKey := client.ObjectKey{
+		Namespace: "test",
+		Name:      "camel-k-puller-test-default",
+	}
+	err = e.Client.Get(e.C, roleBindingKey, &roleBinding)
+	assert.NoError(t, err)
+	assert.Len(t, roleBinding.Subjects, 1)
+}
+
+func getEnvironmentAndDeployment(t *testing.T) (*Environment, *appsv1.Deployment) {
 	e := &Environment{}
 	e.Integration = &v1.Integration{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "test",
+			Name:      "myit",
+		},
 		Status: v1.IntegrationStatus{
 			Phase: v1.IntegrationPhaseDeploying,
 		},
 	}
-	e.Platform = &v1.IntegrationPlatform{}
 
 	deployment := appsv1.Deployment{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "test",
+			Name:      "myit",
+		},
 		Spec: appsv1.DeploymentSpec{
 			Template: corev1.PodTemplateSpec{
 				Spec: corev1.PodSpec{},
@@ -74,8 +116,20 @@ func TestPullSecretDoesNothingWhenNotSetOnPlatform(t *testing.T) {
 	}
 	e.Resources = kubernetes.NewCollection(&deployment)
 
-	trait := newPullSecretTrait()
-	enabled, err := trait.Configure(e)
-	assert.Nil(t, err)
-	assert.False(t, enabled)
+	var err error
+	e.C = context.TODO()
+	e.Client, err = test.NewFakeClient(e.Integration, &deployment)
+	assert.NoError(t, err)
+
+	return e, &deployment
+}
+
+func newFalse() *bool {
+	b := false
+	return &b
+}
+
+func newTrue() *bool {
+	b := true
+	return &b
 }