Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug 1905119: dynamically update controller asset for custom CA bundle #111

Merged
merged 1 commit into from
Feb 2, 2021
Merged

Bug 1905119: dynamically update controller asset for custom CA bundle #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 0 additions & 14 deletions assets/controller.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,6 @@ spec:
value: '1'
- name: AWS_CONFIG_FILE
value: /var/run/secrets/aws/credentials
{{- if .CABundleConfigMap}}
- name: AWS_CA_BUNDLE
value: /etc/ca/ca-bundle.pem
{{- end}}
ports:
- name: healthz
# Due to hostNetwork, this port is open on a node!
Expand All @@ -70,11 +66,6 @@ spec:
- name: bound-sa-token
mountPath: /var/run/secrets/openshift/serviceaccount
readOnly: true
{{- if .CABundleConfigMap}}
- name: ca-bundle
mountPath: /etc/ca
readOnly: true
{{- end}}
- name: socket-dir
mountPath: /var/lib/csi/sockets/pluginproxy/
resources:
Expand Down Expand Up @@ -170,10 +161,5 @@ spec:
- serviceAccountToken:
path: token
audience: openshift
{{- if .CABundleConfigMap}}
- name: ca-bundle
configMap:
name: {{.CABundleConfigMap}}
{{- end}}
- name: socket-dir
emptyDir: {}
14 changes: 0 additions & 14 deletions pkg/generated/bindata.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

72 changes: 48 additions & 24 deletions pkg/operator/starter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
package operator

import (
"bytes"
"context"
"fmt"
"text/template"
"time"

"github.com/openshift/library-go/pkg/controller/factory"
"github.com/openshift/library-go/pkg/operator/events"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kubeclient "k8s.io/client-go/kubernetes"
corev1listers "k8s.io/client-go/listers/core/v1"
"k8s.io/client-go/rest"
"k8s.io/klog/v2"

Expand Down Expand Up @@ -52,6 +52,11 @@ func RunOperator(ctx context.Context, controllerConfig *controllercmd.Controller
configClient := configclient.NewForConfigOrDie(rest.AddUserAgent(controllerConfig.KubeConfig, operatorName))
configInformers := configinformers.NewSharedInformerFactory(configClient, 20*time.Minute)

// Create informer for the ConfigMaps in the openshift-config-managed namespace. This is used to get the custom CA
// bundle to use when accessing the AWS API.
cloudConfigInformer := kubeInformersForNamespaces.InformersFor(cloudConfigNamespace).Core().V1().ConfigMaps()
cloudConfigLister := cloudConfigInformer.Lister().ConfigMaps(cloudConfigNamespace)

// Create GenericOperatorclient. This is used by the library-go controllers created down below
gvr := opv1.SchemeGroupVersion.WithResource("clustercsidrivers")
operatorClient, dynamicInformers, err := goc.NewClusterScopedOperatorClientWithConfigName(controllerConfig.KubeConfig, gvr, instanceName)
Expand Down Expand Up @@ -92,21 +97,25 @@ func RunOperator(ctx context.Context, controllerConfig *controllercmd.Controller
configInformers,
).WithCSIDriverControllerService(
"AWSEBSDriverControllerServiceController",
withCustomCABundle(generated.MustAsset, kubeClient),
generated.MustAsset,
"controller.yaml",
kubeClient,
kubeInformersForNamespaces.InformersFor(defaultNamespace),
configInformers,
csidrivercontrollerservicecontroller.WithSecretHashAnnotationHook(defaultNamespace, secretName, secretInformer),
csidrivercontrollerservicecontroller.WithObservedProxyDeploymentHook(),
withCustomCABundle(cloudConfigLister),
).WithCSIDriverNodeService(
"AWSEBSDriverNodeServiceController",
generated.MustAsset,
"node.yaml",
kubeClient,
kubeInformersForNamespaces.InformersFor(defaultNamespace),
csidrivernodeservicecontroller.WithObservedProxyDaemonSetHook(),
).WithExtraInformers(secretInformer.Informer())
).WithExtraInformers(
secretInformer.Informer(),
cloudConfigInformer.Informer(),
)

if err != nil {
return err
Expand Down Expand Up @@ -143,22 +152,39 @@ type controllerTemplateData struct {
// withCustomCABundle executes the asset as a template to fill out the parts required when using a custom CA bundle.
// The `caBundleConfigMap` parameter specifies the name of the ConfigMap containing the custom CA bundle. If the
// argument supplied is empty, then no custom CA bundle will be used.
func withCustomCABundle(assetFunc func(string) []byte, kubeClient kubeclient.Interface) func(string) []byte {
templateData := controllerTemplateData{}
switch used, err := isCustomCABundleUsed(kubeClient); {
case err != nil:
klog.Fatalf("could not determine if a custom CA bundle is in use: %v", err)
case used:
templateData.CABundleConfigMap = cloudConfigName
}
return func(name string) []byte {
asset := assetFunc(name)
template := template.Must(template.New("template").Parse(string(asset)))
buf := &bytes.Buffer{}
if err := template.Execute(buf, templateData); err != nil {
klog.Fatalf("Failed to execute ")
func withCustomCABundle(cloudConfigLister corev1listers.ConfigMapNamespaceLister) csidrivercontrollerservicecontroller.DeploymentHookFunc {
return func(_ *opv1.OperatorSpec, deployment *appsv1.Deployment) error {
switch used, err := isCustomCABundleUsed(cloudConfigLister); {
case err != nil:
return fmt.Errorf("could not determine if a custom CA bundle is in use: %w", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm: returning an error here will degrade the operator. Is that what we want?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in my opinion that is what we want. But if you think otherwise, it won't hurt my feelings. Since we are using a lister, an error here is either (1) that the informer has been closed or (2) that there are RBAC restrictions preventing access to the ConfigMap. Both of those seem to me to warrant the operator being marked as degraded.

case !used:
return nil
}
deployment.Spec.Template.Spec.Volumes = append(deployment.Spec.Template.Spec.Volumes, corev1.Volume{
Name: "ca-bundle",
VolumeSource: corev1.VolumeSource{
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: corev1.LocalObjectReference{Name: cloudConfigName},
},
},
})
for i := range deployment.Spec.Template.Spec.Containers {
container := &deployment.Spec.Template.Spec.Containers[i]
if container.Name != "csi-driver" {
continue
}
container.Env = append(container.Env, corev1.EnvVar{
Name: "AWS_CA_BUNDLE",
Value: "/etc/ca/ca-bundle.pem",
})
container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{
Name: "ca-bundle",
MountPath: "/etc/ca",
ReadOnly: true,
})
return nil
}
return buf.Bytes()
return fmt.Errorf("could not use custom CA bundle because the csi-driver container is missing from the deployment")
}
}

Expand Down Expand Up @@ -192,10 +218,8 @@ func newCustomCABundleSyncer(
}

// isCustomCABundleUsed returns true if the cloud config ConfigMap exists and contains a custom CA bundle.
func isCustomCABundleUsed(kubeClient kubeclient.Interface) (bool, error) {
cloudConfigCM, err := kubeClient.CoreV1().
ConfigMaps(cloudConfigNamespace).
Get(context.Background(), cloudConfigName, metav1.GetOptions{})
func isCustomCABundleUsed(cloudConfigLister corev1listers.ConfigMapNamespaceLister) (bool, error) {
cloudConfigCM, err := cloudConfigLister.Get(cloudConfigName)
if errors.IsNotFound(err) {
// no cloud config ConfigMap so there is no CA bundle
return false, nil
Expand Down