allow upgraded clusters to use the vulnerable service-ca.crt

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/gonum/graph v0.0.0-20170401004347-50b27dea7ebb
 	github.com/google/go-cmp v0.5.2
 	github.com/google/gofuzz v1.2.0 // indirect
-	github.com/openshift/api v0.0.0-20210629145910-15a1cae1fca8
+	github.com/openshift/api v0.0.0-20210712164256-f428c6b5808e
 	github.com/openshift/build-machinery-go v0.0.0-20210423112049-9415d7ebd33e
 	github.com/openshift/client-go v0.0.0-20210521082421-73d9475a9142
 	github.com/openshift/library-go v0.0.0-20210702104503-39570b4a2ae8

go.sum

@@ -404,8 +404,8 @@ github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQ
 github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0=
 github.com/openshift/api v0.0.0-20210521075222-e273a339932a/go.mod h1:izBmoXbUu3z5kUa4FjZhvekTsyzIWiOoaIgJiZBBMQs=
 github.com/openshift/api v0.0.0-20210616173328-fb4df74c2da9/go.mod h1:izBmoXbUu3z5kUa4FjZhvekTsyzIWiOoaIgJiZBBMQs=
-github.com/openshift/api v0.0.0-20210629145910-15a1cae1fca8 h1:/wwYnC6F/XQJW2DyvoEZRgO2UFExBl9nSQaQbEEgbXA=
-github.com/openshift/api v0.0.0-20210629145910-15a1cae1fca8/go.mod h1:izBmoXbUu3z5kUa4FjZhvekTsyzIWiOoaIgJiZBBMQs=
+github.com/openshift/api v0.0.0-20210712164256-f428c6b5808e h1:Ya4U6Sr9sZhFpu+ySDax4HA//69hZlmlRqoP1hJqyq4=
+github.com/openshift/api v0.0.0-20210712164256-f428c6b5808e/go.mod h1:izBmoXbUu3z5kUa4FjZhvekTsyzIWiOoaIgJiZBBMQs=
 github.com/openshift/build-machinery-go v0.0.0-20210423112049-9415d7ebd33e h1:F7rBobgSjtYL3/zsgDUjlTVx3Z06hdgpoldpDcn7jzc=
 github.com/openshift/build-machinery-go v0.0.0-20210423112049-9415d7ebd33e/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE=
 github.com/openshift/client-go v0.0.0-20210521082421-73d9475a9142 h1:ZHRIMCFIJN1p9LsJt4HQ+akDrys4PrYnXzOWI5LK03I=

manifests/0000_25_kube-controller-manager-operator_01_operator.cr.yaml

@@ -9,3 +9,8 @@ metadata:
     release.openshift.io/create-only: "true"
 spec:
   managementState: Managed
+  # This ensures that freshly installed clusters will use the more secure service CA.
+  # Upgraded clusters will have `false` here.  Newly created clusters will always have had this value as `true`.
+  # This guarantee ensures that new clusters will always be more secure and upgraded clustered will always be
+  # less secure until a cluster-admin opts-in.
+  useMoreSecureServiceCA: true

pkg/operator/starter.go

@@ -55,6 +55,7 @@ func RunOperator(ctx context.Context, cc *controllercmd.ControllerContext) error
 	if err != nil {
 		return err
 	}
+	operatorLister := dynamicInformers.ForResource(operatorv1.GroupVersion.WithResource("kubecontrollermanagers")).Lister()
 
 	resourceSyncController, err := resourcesynccontroller.NewResourceSyncController(
 		operatorClient,
@@ -109,6 +110,7 @@ func RunOperator(ctx context.Context, cc *controllercmd.ControllerContext) error
 		os.Getenv("TOOLS_IMAGE"),
 		kubeInformersForNamespaces,
 		operatorClient,
+		operatorLister,
 		kubeClient,
 		configInformers.Config().V1().Infrastructures(),
 		cc.EventRecorder,

pkg/operator/targetconfigcontroller/targetconfigcontroller.go

@@ -12,15 +12,16 @@ import (
 	"time"
 
 	"github.com/ghodss/yaml"
-
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/kubernetes"
 	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	corev1listers "k8s.io/client-go/listers/core/v1"
+	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/util/cert"
 
 	kubecontrolplanev1 "github.com/openshift/api/kubecontrolplane/v1"
@@ -50,6 +51,7 @@ type TargetConfigController struct {
 	toolsImagePullSpec              string
 
 	operatorClient v1helpers.StaticPodOperatorClient
+	operatorLister cache.GenericLister
 
 	kubeClient          kubernetes.Interface
 	configMapLister     corev1listers.ConfigMapLister
@@ -61,6 +63,7 @@ func NewTargetConfigController(
 	targetImagePullSpec, operatorImagePullSpec, clusterPolicyControllerPullSpec, toolsImagePullSpec string,
 	kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces,
 	operatorClient v1helpers.StaticPodOperatorClient,
+	operatorLister cache.GenericLister,
 	kubeClient kubernetes.Interface,
 	infrastuctureInformer configv1informers.InfrastructureInformer,
 	eventRecorder events.Recorder,
@@ -75,6 +78,7 @@ func NewTargetConfigController(
 		secretLister:        kubeInformersForNamespaces.SecretLister(),
 		infrastuctureLister: infrastuctureInformer.Lister(),
 		operatorClient:      operatorClient,
+		operatorLister:      operatorLister,
 		kubeClient:          kubeClient,
 	}
 
@@ -121,7 +125,24 @@ func (c TargetConfigController) sync(ctx context.Context, syncCtx factory.SyncCo
 		return err
 	}
 
-	requeue, err := createTargetConfigController(ctx, syncCtx, c, operatorSpec)
+	// TODO this entire block should become a configobserver, but that requires changes to the observedconfig format.
+	//  I would do that in 4.9, not 4.8.
+	// we need to get at the content of the kcm operator resource itself.  The operatorClient should be improved to return this
+	uncastOperator, err := c.operatorLister.Get("cluster")
+	if err != nil {
+		return err
+	}
+	rawOperator := uncastOperator.(*unstructured.Unstructured)
+	kcmOperator := &operatorv1.KubeControllerManager{}
+	if err := runtime.DefaultUnstructuredConverter.FromUnstructured(rawOperator.UnstructuredContent(), kcmOperator); err != nil {
+		return err
+	}
+	// because of the way the injector ratchets and only allows injecting the "secure" approach, we only need see a positive value.
+	// in the case of an upgraded cluster, this value is empty to begin and may eventually become "good".
+	// in the case of a new cluster, the first instance ever created will be "good", so there is no possibility to accidentally create a "bad" set of flags.
+	useSecureServiceCA := kcmOperator.Spec.UseMoreSecureServiceCA
+
+	requeue, err := createTargetConfigController(ctx, syncCtx, c, operatorSpec, useSecureServiceCA)
 	if err != nil {
 		return err
 	}
@@ -168,7 +189,7 @@ func isRequiredConfigPresent(config []byte) error {
 }
 
 // createTargetConfigController takes care of synchronizing (not upgrading) the thing we're managing.
-func createTargetConfigController(ctx context.Context, syncCtx factory.SyncContext, c TargetConfigController, operatorSpec *operatorv1.StaticPodOperatorSpec) (bool, error) {
+func createTargetConfigController(ctx context.Context, syncCtx factory.SyncContext, c TargetConfigController, operatorSpec *operatorv1.StaticPodOperatorSpec, useSecureServiceCA bool) (bool, error) {
 	errors := []error{}
 
 	_, _, err := manageKubeControllerManagerConfig(ctx, c.kubeClient.CoreV1(), syncCtx.Recorder(), operatorSpec)
@@ -229,7 +250,7 @@ func createTargetConfigController(ctx context.Context, syncCtx factory.SyncConte
 		}
 	}
 
-	_, _, err = managePod(ctx, c.kubeClient.CoreV1(), c.kubeClient.CoreV1(), syncCtx.Recorder(), operatorSpec, c.targetImagePullSpec, c.operatorImagePullSpec, c.clusterPolicyControllerPullSpec, addServingServiceCAToTokenSecrets)
+	_, _, err = managePod(ctx, c.kubeClient.CoreV1(), c.kubeClient.CoreV1(), syncCtx.Recorder(), operatorSpec, c.targetImagePullSpec, c.operatorImagePullSpec, c.clusterPolicyControllerPullSpec, addServingServiceCAToTokenSecrets, useSecureServiceCA)
 	if err != nil {
 		errors = append(errors, fmt.Errorf("%q: %v", "configmap/kube-controller-manager-pod", err))
 	}
@@ -404,7 +425,7 @@ func manageRecycler(ctx context.Context, configMapsGetter corev1client.ConfigMap
 	return resourceapply.ApplyConfigMap(ctx, configMapsGetter, recorder, requiredCM)
 }
 
-func managePod(ctx context.Context, configMapsGetter corev1client.ConfigMapsGetter, secretsGetter corev1client.SecretsGetter, recorder events.Recorder, operatorSpec *operatorv1.StaticPodOperatorSpec, imagePullSpec, operatorImagePullSpec, clusterPolicyControllerPullSpec string, addServingServiceCAToTokenSecrets bool) (*corev1.ConfigMap, bool, error) {
+func managePod(ctx context.Context, configMapsGetter corev1client.ConfigMapsGetter, secretsGetter corev1client.SecretsGetter, recorder events.Recorder, operatorSpec *operatorv1.StaticPodOperatorSpec, imagePullSpec, operatorImagePullSpec, clusterPolicyControllerPullSpec string, addServingServiceCAToTokenSecrets, useSecureServiceCA bool) (*corev1.ConfigMap, bool, error) {
 	required := resourceread.ReadPodV1OrDie(bindata.MustAsset("assets/kube-controller-manager/pod.yaml"))
 	// TODO: If the image pull spec is not specified, the "${IMAGE}" will be used as value and the pod will fail to start.
 	images := map[string]string{
@@ -526,6 +547,18 @@ func managePod(ctx context.Context, configMapsGetter corev1client.ConfigMapsGett
 		required.Spec.Containers[i].Env = append(container.Env, proxyEnvVars...)
 	}
 
+	// set the env var to indicate that we want this vulnerable behavior.
+	if !useSecureServiceCA {
+		for i, container := range required.Spec.Containers {
+			// if the KCM is ever started without this env var, then some namespaces will forever have the secure
+			// (updated) version of the ca bundle content.
+			required.Spec.Containers[i].Env = append(container.Env, corev1.EnvVar{
+				Name:  "OPENSHIFT_USE_VULNERABLE_LEGACY_SERVICE_CA_CRT",
+				Value: "true",
+			})
+		}
+	}
+
 	if addServingServiceCAToTokenSecrets {
 		// Ensure the addition of serving service ca to token secrets by setting the environment
 		// variable that will enable the behavior in the controller manager.