revert trustbundle propagation in sb lifecycle Do (#8985)

* revert trustbundle propagation in sb lifecycle Do

* see if we can set the bundle CMs directly for apiserversource

* update apiserversource_test since we now update the RA immediately

* better names for the "bundle" variables

* pass the propagated trust bundles directly to the integration Deployment

* refactor MakeDeploymentSpec so we don't pass both the lister and the list

Author: maschmid

pkg/apis/sources/v1/sinkbinding_lifecycle.go

@@ -23,7 +23,6 @@ import (
 	"strings"
 
 	"go.uber.org/zap"
-	"k8s.io/client-go/kubernetes"
 	corev1listers "k8s.io/client-go/listers/core/v1"
 
 	corev1 "k8s.io/api/core/v1"
@@ -215,30 +214,13 @@ func (sb *SinkBinding) Do(ctx context.Context, ps *duckv1.WithPod) {
 			Value: ceOverrides,
 		})
 	}
-	gvk := schema.GroupVersionKind{
-		Group:   SchemeGroupVersion.Group,
-		Version: SchemeGroupVersion.Version,
-		Kind:    "SinkBinding",
-	}
-	bundles, err := eventingtls.PropagateTrustBundles(ctx, getKubeClient(ctx), GetTrustBundleConfigMapLister(ctx), gvk, sb)
+
+	pss, err := eventingtls.AddTrustBundleVolumes(GetTrustBundleConfigMapLister(ctx), sb, &ps.Spec.Template.Spec)
 	if err != nil {
-		logging.FromContext(ctx).Errorw("Failed to propagate trust bundles", zap.Error(err))
-	}
-	if len(bundles) > 0 {
-		pss, err := eventingtls.AddTrustBundleVolumesFromConfigMaps(bundles, &ps.Spec.Template.Spec)
-		if err != nil {
-			logging.FromContext(ctx).Errorw("Failed to add trust bundle volumes from configmaps %s/%s: %+v", zap.Error(err))
-			return
-		}
-		ps.Spec.Template.Spec = *pss
-	} else {
-		pss, err := eventingtls.AddTrustBundleVolumes(GetTrustBundleConfigMapLister(ctx), sb, &ps.Spec.Template.Spec)
-		if err != nil {
-			logging.FromContext(ctx).Errorw("Failed to add trust bundle volumes %s/%s: %+v", zap.Error(err))
-			return
-		}
-		ps.Spec.Template.Spec = *pss
+		logging.FromContext(ctx).Errorw("Failed to add trust bundle volumes %s/%s: %+v", zap.Error(err))
+		return
 	}
+	ps.Spec.Template.Spec = *pss
 
 	if sb.Status.OIDCTokenSecretName != nil {
 		ps.Spec.Template.Spec.Volumes = append(ps.Spec.Template.Spec.Volumes, corev1.Volume{
@@ -348,20 +330,6 @@ func (sb *SinkBinding) Undo(ctx context.Context, ps *duckv1.WithPod) {
 	}
 }
 
-type kubeClientKey struct{}
-
-func WithKubeClient(ctx context.Context, k kubernetes.Interface) context.Context {
-	return context.WithValue(ctx, kubeClientKey{}, k)
-}
-
-func getKubeClient(ctx context.Context) kubernetes.Interface {
-	k := ctx.Value(kubeClientKey{})
-	if k == nil {
-		panic("No Kube client found in context.")
-	}
-	return k.(kubernetes.Interface)
-}
-
 type configMapListerKey struct{}
 
 func WithTrustBundleConfigMapLister(ctx context.Context, lister corev1listers.ConfigMapLister) context.Context {

pkg/apis/sources/v1/sinkbinding_lifecycle_test.go

@@ -32,7 +32,6 @@ import (
 	"knative.dev/pkg/apis"
 	duckv1 "knative.dev/pkg/apis/duck/v1"
 	"knative.dev/pkg/client/injection/ducks/duck/v1/addressable"
-	kubeclient "knative.dev/pkg/client/injection/kube/client/fake"
 	configmapinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/fake"
 	fakedynamicclient "knative.dev/pkg/injection/clients/dynamicclient/fake"
 	"knative.dev/pkg/resolver"
@@ -929,7 +928,6 @@ func TestSinkBindingDo(t *testing.T) {
 			}
 			ctx = WithURIResolver(ctx, r)
 			ctx = WithTrustBundleConfigMapLister(ctx, configmapinformer.Get(ctx).Lister())
-			ctx = WithKubeClient(ctx, kubeclient.Get(ctx))
 
 			for _, cm := range test.configMaps {
 				_ = configmapinformer.Get(ctx).Informer().GetIndexer().Add(cm)

pkg/reconciler/apiserversource/apiserversource.go

@@ -162,13 +162,14 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, source *v1.ApiServerSour
 		}
 	}
 
-	if err := r.propagateTrustBundles(ctx, source); err != nil {
+	trustBundleConfigMaps, err := r.propagateTrustBundles(ctx, source)
+	if err != nil {
 		return err
 	}
 
 	// An empty selector targets all namespaces.
 	allNamespaces := isEmptySelector(source.Spec.NamespaceSelector)
-	ra, err := r.createReceiveAdapter(ctx, source, sinkAddr, namespaces, allNamespaces)
+	ra, err := r.createReceiveAdapter(ctx, source, sinkAddr, namespaces, allNamespaces, trustBundleConfigMaps)
 	if err != nil {
 		logging.FromContext(ctx).Errorw("Unable to create the receive adapter", zap.Error(err))
 		return err
@@ -228,7 +229,7 @@ func isEmptySelector(selector *metav1.LabelSelector) bool {
 	return false
 }
 
-func (r *Reconciler) createReceiveAdapter(ctx context.Context, src *v1.ApiServerSource, sinkAddr *duckv1.Addressable, namespaces []string, allNamespaces bool) (*appsv1.Deployment, error) {
+func (r *Reconciler) createReceiveAdapter(ctx context.Context, src *v1.ApiServerSource, sinkAddr *duckv1.Addressable, namespaces []string, allNamespaces bool, trustBundleConfigMaps []*corev1.ConfigMap) (*appsv1.Deployment, error) {
 	// TODO: missing.
 	// if err := checkResourcesStatus(src); err != nil {
 	// 	return nil, err
@@ -258,11 +259,20 @@ func (r *Reconciler) createReceiveAdapter(ctx context.Context, src *v1.ApiServer
 		return nil, err
 	}
 
-	podTemplate, err := eventingtls.AddTrustBundleVolumes(r.trustBundleConfigMapLister, src, &expected.Spec.Template.Spec)
-	if err != nil {
-		return nil, fmt.Errorf("failed to add trust bundle volumes: %w", err)
+	if len(trustBundleConfigMaps) > 0 {
+		podTemplate, err := eventingtls.AddTrustBundleVolumesFromConfigMaps(trustBundleConfigMaps, &expected.Spec.Template.Spec)
+		if err != nil {
+			return nil, fmt.Errorf("failed to add trust bundle volumes from configmaps: %w", err)
+		}
+		expected.Spec.Template.Spec = *podTemplate
+	} else {
+		podTemplate, err := eventingtls.AddTrustBundleVolumes(r.trustBundleConfigMapLister, src, &expected.Spec.Template.Spec)
+
+		if err != nil {
+			return nil, fmt.Errorf("failed to add trust bundle volumes: %w", err)
+		}
+		expected.Spec.Template.Spec = *podTemplate
 	}
-	expected.Spec.Template.Spec = *podTemplate
 
 	ra, err := r.kubeClientSet.AppsV1().Deployments(src.Namespace).Get(ctx, expected.Name, metav1.GetOptions{})
 	if apierrors.IsNotFound(err) {
@@ -472,12 +482,11 @@ func (r *Reconciler) createOIDCRoleBinding(ctx context.Context, source *v1.ApiSe
 	return nil
 }
 
-func (r *Reconciler) propagateTrustBundles(ctx context.Context, source *v1.ApiServerSource) error {
+func (r *Reconciler) propagateTrustBundles(ctx context.Context, source *v1.ApiServerSource) ([]*corev1.ConfigMap, error) {
 	gvk := schema.GroupVersionKind{
 		Group:   v1.SchemeGroupVersion.Group,
 		Version: v1.SchemeGroupVersion.Version,
 		Kind:    "ApiServerSource",
 	}
-	_, err := eventingtls.PropagateTrustBundles(ctx, r.kubeClientSet, r.trustBundleConfigMapLister, gvk, source)
-	return err
+	return eventingtls.PropagateTrustBundles(ctx, r.kubeClientSet, r.trustBundleConfigMapLister, gvk, source)
 }

pkg/reconciler/apiserversource/apiserversource_test.go

@@ -256,8 +256,12 @@ func TestReconcile(t *testing.T) {
 				}),
 			),
 		},
+		WantUpdates: []clientgotesting.UpdateActionImpl{{
+			Object: makeAvailableReceiveAdapter(t, withTrustBundle("bundle")),
+		}},
 		WantEvents: []string{
 			Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", sourceName),
+			Eventf(corev1.EventTypeNormal, "ApiServerSourceDeploymentUpdated", `Deployment "apiserversource-test-apiserver-source-1234" updated`),
 		},
 		WantPatches: []clientgotesting.PatchActionImpl{
 			patchFinalizers(sourceName, testNS),

pkg/reconciler/integration/sink/integrationsink.go

@@ -101,13 +101,14 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, sink *sinks.IntegrationS
 	logger := logging.FromContext(ctx)
 
 	logger.Debugw("Reconciling Trust Bundles")
-	if err := r.reconcileIntegrationSinkTrustBundles(ctx, sink); err != nil {
+	trustBundleConfigMaps, err := r.reconcileIntegrationSinkTrustBundles(ctx, sink)
+	if err != nil {
 		logger.Errorw("Error reconciling Trust Bundles", zap.Error(err))
 		return err
 	}
 
 	logger.Debugw("Reconciling IntegrationSink Certificate")
-	_, err := r.reconcileIntegrationSinkCertificate(ctx, sink)
+	_, err = r.reconcileIntegrationSinkCertificate(ctx, sink)
 	if err != nil {
 		logging.FromContext(ctx).Errorw("Error reconciling Certificate", zap.Error(err))
 		return err
@@ -121,7 +122,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, sink *sinks.IntegrationS
 	}
 
 	logger.Debugw("Reconciling IntegrationSink Deployment")
-	_, err = r.reconcileDeployment(ctx, sink, r.authProxyImage, featureFlags)
+	_, err = r.reconcileDeployment(ctx, sink, r.authProxyImage, featureFlags, trustBundleConfigMaps)
 	if err != nil {
 		logging.FromContext(ctx).Errorw("Error reconciling Pod", zap.Error(err))
 		return err
@@ -148,8 +149,15 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, sink *sinks.IntegrationS
 	return newReconciledNormal(sink.Namespace, sink.Name)
 }
 
-func (r *Reconciler) reconcileDeployment(ctx context.Context, sink *sinks.IntegrationSink, authProxyImage string, featureFlags feature.Flags) (*v1.Deployment, error) {
-	expected, err := resources.MakeDeploymentSpec(sink, authProxyImage, featureFlags, r.trustBundleConfigMapLister)
+func (r *Reconciler) reconcileDeployment(ctx context.Context, sink *sinks.IntegrationSink, authProxyImage string, featureFlags feature.Flags, trustBundleConfigMaps []*corev1.ConfigMap) (*v1.Deployment, error) {
+	if len(trustBundleConfigMaps) == 0 {
+		var err error
+		trustBundleConfigMaps, err = r.trustBundleConfigMapLister.ConfigMaps(sink.Namespace).List(eventingtls.TrustBundleSelector)
+		if err != nil {
+			return nil, fmt.Errorf("failed to list trust bundle ConfigMaps: %w", err)
+		}
+	}
+	expected, err := resources.MakeDeploymentSpec(sink, authProxyImage, featureFlags, trustBundleConfigMaps)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create deployment template: %w", err)
 	}
@@ -282,20 +290,21 @@ func (r *Reconciler) deleteIntegrationSinkCertificate(ctx context.Context, sink
 	return nil
 }
 
-func (r *Reconciler) reconcileIntegrationSinkTrustBundles(ctx context.Context, sink *sinks.IntegrationSink) error {
+func (r *Reconciler) reconcileIntegrationSinkTrustBundles(ctx context.Context, sink *sinks.IntegrationSink) ([]*corev1.ConfigMap, error) {
 	gvk := schema.GroupVersionKind{
 		Group:   sinks.SchemeGroupVersion.Group,
 		Version: sinks.SchemeGroupVersion.Version,
 		Kind:    "IntegrationSink",
 	}
 
-	if _, err := eventingtls.PropagateTrustBundles(ctx, r.kubeClientSet, r.trustBundleConfigMapLister, gvk, sink); err != nil {
+	trustBundleConfigMaps, err := eventingtls.PropagateTrustBundles(ctx, r.kubeClientSet, r.trustBundleConfigMapLister, gvk, sink)
+	if err != nil {
 		sink.Status.MarkFailedTrustBundlePropagation("FailedTrustBundlePropagation", err.Error())
-		return err
+		return nil, err
 	}
 	sink.Status.MarkTrustBundlePropagated()
 
-	return nil
+	return trustBundleConfigMaps, nil
 }
 
 func (r *Reconciler) reconcileAddress(ctx context.Context, sink *sinks.IntegrationSink) error {

pkg/reconciler/integration/sink/resources/container_image.go

@@ -27,7 +27,6 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
-	corev1listers "k8s.io/client-go/listers/core/v1"
 	"k8s.io/utils/ptr"
 	commonv1a1 "knative.dev/eventing/pkg/apis/common/integration/v1alpha1"
 	"knative.dev/eventing/pkg/apis/feature"
@@ -46,7 +45,7 @@ const (
 	AuthProxyRolebindingName = "eventing-auth-proxy"
 )
 
-func MakeDeploymentSpec(sink *v1alpha1.IntegrationSink, authProxyImage string, featureFlags feature.Flags, trustBundleConfigmapLister corev1listers.ConfigMapLister) (*appsv1.Deployment, error) {
+func MakeDeploymentSpec(sink *v1alpha1.IntegrationSink, authProxyImage string, featureFlags feature.Flags, trustBundleConfigMaps []*corev1.ConfigMap) (*appsv1.Deployment, error) {
 	probesPort := int32(8080)
 	probesScheme := corev1.URISchemeHTTP
 	if featureFlags.IsStrictTransportEncryption() {
@@ -202,7 +201,7 @@ func MakeDeploymentSpec(sink *v1alpha1.IntegrationSink, authProxyImage string, f
 		})
 
 		// add trustbundles directly, so auth-proxies tokenverifier does not need the trustbundleconfigmap lister for oidc discovery
-		podspec, err := eventingtls.AddTrustBundleVolumes(trustBundleConfigmapLister, deploy, &deploy.Spec.Template.Spec)
+		podspec, err := eventingtls.AddTrustBundleVolumesFromConfigMaps(trustBundleConfigMaps, &deploy.Spec.Template.Spec)
 		if err != nil {
 			return nil, fmt.Errorf("failed to add trust bundle volumes: %w", err)
 		}

pkg/reconciler/sinkbinding/controller.go

@@ -150,9 +150,8 @@ func NewController(
 		trustBundleConfigMapLister: trustBundleConfigMapLister,
 	}
 
-	k8s := kubeclient.Get(ctx)
 	c.WithContext = func(ctx context.Context, b psbinding.Bindable) (context.Context, error) {
-		return v1.WithKubeClient(v1.WithTrustBundleConfigMapLister(v1.WithURIResolver(ctx, sbResolver), trustBundleConfigMapLister), k8s), nil
+		return v1.WithTrustBundleConfigMapLister(v1.WithURIResolver(ctx, sbResolver), trustBundleConfigMapLister), nil
 	}
 	c.Tracker = impl.Tracker
 	c.Factory = &duck.CachedInformerFactory{
@@ -226,10 +225,9 @@ func ListAll(ctx context.Context, handler cache.ResourceEventHandler) psbinding.
 
 func WithContextFactory(ctx context.Context, lister corev1listers.ConfigMapLister, handler func(types.NamespacedName)) psbinding.BindableContext {
 	r := resolver.NewURIResolverFromTracker(ctx, tracker.New(handler, controller.GetTrackerLease(ctx)))
-	k := kubeclient.Get(ctx)
 
 	return func(ctx context.Context, b psbinding.Bindable) (context.Context, error) {
-		return v1.WithKubeClient(v1.WithTrustBundleConfigMapLister(v1.WithURIResolver(ctx, r), lister), k), nil
+		return v1.WithTrustBundleConfigMapLister(v1.WithURIResolver(ctx, r), lister), nil
 	}
 }