Skip to content

STOR-2809: Central TLS Profile consistency #579

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

Open
dfajmon wants to merge 1 commit into
base: main
Choose a base branch
from
Open

STOR-2809: Central TLS Profile consistency #579

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.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 2 additions & 0 deletions assets/templates/diskmaker-discovery-daemonset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ spec:
- --upstream=http://127.0.0.1:8383/
- --tls-cert-file=/etc/tls/private/tls.crt
- --tls-private-key-file=/etc/tls/private/tls.key
- --tls-cipher-suites=${TLS_CIPHER_SUITES}
- --tls-min-version=${TLS_MIN_VERSION}
image: ${RBAC_PROXY_IMAGE}
imagePullPolicy: IfNotPresent
name: kube-rbac-proxy
Expand Down
2 changes: 2 additions & 0 deletions assets/templates/diskmaker-manager-daemonset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ spec:
- --upstream=http://127.0.0.1:8383/
- --tls-cert-file=/etc/tls/private/tls.crt
- --tls-private-key-file=/etc/tls/private/tls.key
- --tls-cipher-suites=${TLS_CIPHER_SUITES}
- --tls-min-version=${TLS_MIN_VERSION}
image: ${RBAC_PROXY_IMAGE}
imagePullPolicy: IfNotPresent
name: kube-rbac-proxy
Expand Down
2 changes: 2 additions & 0 deletions cmd/local-storage-operator/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
_ "k8s.io/client-go/plugin/pkg/client/auth"
"k8s.io/klog/v2"

configv1 "github.com/openshift/api/config/v1"
localv1 "github.com/openshift/local-storage-operator/api/v1"
localv1alpha1 "github.com/openshift/local-storage-operator/api/v1alpha1"
"github.com/openshift/local-storage-operator/pkg/common"
Expand Down Expand Up @@ -56,6 +57,7 @@ var (

func init() {
utilruntime.Must(clientgoscheme.AddToScheme(scheme))
utilruntime.Must(configv1.AddToScheme(scheme))
utilruntime.Must(localv1.AddToScheme(scheme))
utilruntime.Must(localv1alpha1.AddToScheme(scheme))
utilruntime.Must(monitoringv1.AddToScheme(scheme))
Expand Down
3 changes: 2 additions & 1 deletion config/manifests/stable/local-storage-operator.clusterserviceversion.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ metadata:
features.operators.openshift.io/disconnected: "true"
features.operators.openshift.io/fips-compliant: "true"
features.operators.openshift.io/proxy-aware: "true"
features.operators.openshift.io/tls-profiles: "false"
features.operators.openshift.io/tls-profiles: "true"
features.operators.openshift.io/csi: "false"
features.operators.openshift.io/token-auth-aws: "false"
features.operators.openshift.io/token-auth-azure: "false"
Expand Down Expand Up @@ -382,6 +382,7 @@ spec:
- config.openshift.io
resources:
- infrastructures
- apiservers
verbs:
- get
- list
Expand Down
2 changes: 2 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ require (
github.com/go-openapi/swag/typeutils v0.25.1 // indirect
github.com/go-openapi/swag/yamlutils v0.25.1 // indirect
github.com/google/btree v1.1.3 // indirect
github.com/imdario/mergo v0.3.7 // indirect
github.com/robfig/cron v1.2.0 // indirect
github.com/x448/float16 v0.8.4 // indirect
go.opentelemetry.io/otel v1.38.0 // indirect
go.opentelemetry.io/otel/trace v1.38.0 // indirect
Expand Down
4 changes: 4 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79/go.mod h1:Fecb
github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=
github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
github.com/ianlancetaylor/demangle v0.0.0-20240312041847-bd984b5ce465/go.mod h1:gx7rwoVhcfuVKG5uya9Hs3Sxj7EIvldVofAWIUtGouw=
github.com/imdario/mergo v0.3.7 h1:Y+UAYTZ7gDEuOfhxKWy+dvb5dRQ6rJjFSdX2HZY1/gI=
github.com/imdario/mergo v0.3.7/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=
Expand Down Expand Up @@ -354,6 +356,8 @@ github.com/prometheus/procfs v0.0.2/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsT
github.com/prometheus/procfs v0.0.8/go.mod h1:7Qr8sr6344vo1JqZ6HhLceV9o3AJ1Ff+GxbHq6oeK9A=
github.com/prometheus/procfs v0.19.2 h1:zUMhqEW66Ex7OXIiDkll3tl9a1ZdilUOd/F6ZXw4Vws=
github.com/prometheus/procfs v0.19.2/go.mod h1:M0aotyiemPhBCM0z5w87kL22CxfcH05ZpYlu+b4J7mw=
github.com/robfig/cron v1.2.0 h1:ZjScXvvxeQ63Dbyxy76Fj3AT3Ut0aKsyd2/tl3DTMuQ=
github.com/robfig/cron v1.2.0/go.mod h1:JGuDeoQd7Z6yL4zQhZ3OPEVHB7fL6Ka6skscFHfmt2k=
github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4=
github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o=
Expand Down
70 changes: 57 additions & 13 deletions pkg/controllers/localvolumediscovery/localvolumediscovery_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,22 @@ import (
"fmt"
"time"

configv1 "github.com/openshift/api/config/v1"
operatorv1 "github.com/openshift/api/operator/v1"
"github.com/openshift/library-go/pkg/operator/resource/resourceread"
localv1alpha1 "github.com/openshift/local-storage-operator/api/v1alpha1"
"github.com/openshift/local-storage-operator/assets"
"github.com/openshift/local-storage-operator/pkg/common"
"github.com/openshift/local-storage-operator/pkg/controllers/nodedaemon"
"github.com/openshift/local-storage-operator/pkg/localmetrics"
lsotls "github.com/openshift/local-storage-operator/pkg/tls"
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"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/record"
v1helper "k8s.io/component-helpers/scheduling/corev1"
"k8s.io/klog/v2"
ctrl "sigs.k8s.io/controller-runtime"
Expand All @@ -55,8 +58,10 @@ const (
type LocalVolumeDiscoveryReconciler struct {
// This client, initialized using mgr.Client() above, is a split client
// that reads objects from the cache and writes to the apiserver
Client client.Client
Scheme *runtime.Scheme
Client client.Client
Scheme *runtime.Scheme
Recorder record.EventRecorder
lastObservedTLSConfig map[string]interface{}
}

// Reconcile reads that state of the cluster for a LocalVolumeDiscovery object and makes changes based on the state read
Expand Down Expand Up @@ -88,10 +93,19 @@ func (r *LocalVolumeDiscoveryReconciler) Reconcile(ctx context.Context, request
return ctrl.Result{}, err
}

tlsMinVersion, tlsCipherSuites, observedTLSConfig, err := lsotls.GetTLSProfileValues(ctx, r.Client, r.lastObservedTLSConfig)
if err != nil {
klog.Warningf("failed to get cluster TLS profile, proceeding with kube-rbac-proxy defaults: %v", err)
} else {
r.lastObservedTLSConfig = observedTLSConfig
}

diskMakerDSMutateFn := getDiskMakerDiscoveryDSMutateFn(request, instance.Spec.Tolerations,
getEnvVars(instance.Name, string(instance.UID)),
getOwnerRefs(instance),
instance.Spec.NodeSelector)
instance.Spec.NodeSelector,
tlsMinVersion,
tlsCipherSuites)
ds, opResult, err := nodedaemon.CreateOrUpdateDaemonset(ctx, r.Client, diskMakerDSMutateFn)
if err != nil {
message := fmt.Sprintf("failed to create discovery daemonset. Error %+v", err)
Expand Down Expand Up @@ -151,18 +165,20 @@ func getDiskMakerDiscoveryDSMutateFn(request reconcile.Request,
tolerations []corev1.Toleration,
envVars []corev1.EnvVar,
ownerRefs []metav1.OwnerReference,
nodeSelector *corev1.NodeSelector) func(*appsv1.DaemonSet) error {
nodeSelector *corev1.NodeSelector,
tlsMinVersion string,
tlsCipherSuites string) func(*appsv1.DaemonSet) error {

return func(ds *appsv1.DaemonSet) error {
// read template for default values
dsBytes, err := assets.ReadFileAndReplace(
common.DiskMakerDiscoveryDaemonSetTemplate,
[]string{
"${OBJECT_NAMESPACE}", request.Namespace,
"${CONTAINER_IMAGE}", common.GetDiskMakerImage(),
"${RBAC_PROXY_IMAGE}", common.GetKubeRBACProxyImage(),
},
)
pairs := []string{
"${OBJECT_NAMESPACE}", request.Namespace,
"${CONTAINER_IMAGE}", common.GetDiskMakerImage(),
"${RBAC_PROXY_IMAGE}", common.GetKubeRBACProxyImage(),
"${TLS_MIN_VERSION}", tlsMinVersion,
"${TLS_CIPHER_SUITES}", tlsCipherSuites,
}

dsBytes, err := assets.ReadFileAndReplace(common.DiskMakerDiscoveryDaemonSetTemplate, pairs)
if err != nil {
return err
}
Expand Down Expand Up @@ -290,8 +306,36 @@ func getEnvVars(objName, uid string) []corev1.EnvVar {

// SetupWithManager sets up the controller with the Manager.
func (r *LocalVolumeDiscoveryReconciler) SetupWithManager(mgr ctrl.Manager) error {
watchNamespace, err := common.GetWatchNamespace()
if err != nil {
return err
}
r.Recorder = mgr.GetEventRecorderFor(DiskMakerDiscovery)

// When the cluster TLS profile changes, re-reconcile all LocalVolumeDiscovery objects.
enqueueAllDiscoveries := handler.EnqueueRequestsFromMapFunc(
func(ctx context.Context, obj client.Object) []reconcile.Request {
lvdList := &localv1alpha1.LocalVolumeDiscoveryList{}
if err := r.Client.List(ctx, lvdList, client.InNamespace(watchNamespace)); err != nil {
klog.Errorf("failed to list LocalVolumeDiscoveries on APIServer change: %v", err)
return nil
}
requests := make([]reconcile.Request, 0, len(lvdList.Items))
for _, lvd := range lvdList.Items {
requests = append(requests, reconcile.Request{
NamespacedName: types.NamespacedName{
Namespace: lvd.Namespace,
Name: lvd.Name,
},
})
}
return requests
})

return ctrl.NewControllerManagedBy(mgr).
For(&localv1alpha1.LocalVolumeDiscovery{}).
Watches(&appsv1.DaemonSet{}, handler.EnqueueRequestForOwner(mgr.GetScheme(), mgr.GetRESTMapper(), &localv1alpha1.LocalVolumeDiscovery{})).
// watch cluster TLS profile changes
Watches(&configv1.APIServer{}, enqueueAllDiscoveries).
Complete(r)
}
39 changes: 39 additions & 0 deletions pkg/controllers/nodedaemon/daemonset_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

// "reflect"
Expand Down Expand Up @@ -38,6 +40,43 @@ func TestMutateAggregatedSpecWithNilNodeSelector(t *testing.T) {
assert.NotNilf(t, ds.Spec.Template.Spec.Affinity, "DaemonSet affinity should not be nil if nodeSelector is not nil")
}

func TestGetDiskMakerDSMutateFnTLSArgs(t *testing.T) {
tlsMinVersion := "VersionTLS12"
tlsCipherSuites := "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256"

request := reconcile.Request{
NamespacedName: types.NamespacedName{Namespace: "test-ns"},
}
mutateFn := getDiskMakerDSMutateFn(request, nil, nil, nil, "hash", tlsMinVersion, tlsCipherSuites)
ds := &appsv1.DaemonSet{}
err := mutateFn(ds)
assert.NoError(t, err, "mutateFn should not return an error")

var proxyContainer *corev1.Container
for i := range ds.Spec.Template.Spec.Containers {
if ds.Spec.Template.Spec.Containers[i].Name == "kube-rbac-proxy" {
proxyContainer = &ds.Spec.Template.Spec.Containers[i]
break
}
}
assert.NotNilf(t, proxyContainer, "kube-rbac-proxy container should be present")
assert.Containsf(t, proxyContainer.Args, "--tls-min-version="+tlsMinVersion,
"kube-rbac-proxy args should contain --tls-min-version")
assert.Containsf(t, proxyContainer.Args, "--tls-cipher-suites="+tlsCipherSuites,
"kube-rbac-proxy args should contain --tls-cipher-suites")
}

func TestGetDiskMakerDSMutateFnNoTLSArgs(t *testing.T) {
request := reconcile.Request{
NamespacedName: types.NamespacedName{Namespace: "test-ns"},
}
// Empty TLS values (e.g. when APIServer is unreachable): mutateFn should still succeed.
mutateFn := getDiskMakerDSMutateFn(request, nil, nil, nil, "hash", "", "")
ds := &appsv1.DaemonSet{}
err := mutateFn(ds)
assert.NoError(t, err, "mutateFn should not return an error with empty TLS values")
}

func TestMutateAggregatedSpecTemplates(t *testing.T) {
// Generate DaemonSet template by reading yaml asset
dsBytes, err := assets.ReadFileAndReplace(
Expand Down
22 changes: 12 additions & 10 deletions pkg/controllers/nodedaemon/daemonsets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,19 +63,21 @@ func getDiskMakerDSMutateFn(
ownerRefs []metav1.OwnerReference,
nodeSelector *corev1.NodeSelector,
dataHash string,
tlsMinVersion string,
tlsCipherSuites string,
) func(*appsv1.DaemonSet) error {

return func(ds *appsv1.DaemonSet) error {
// read template for default values
dsBytes, err := assets.ReadFileAndReplace(
common.DiskMakerManagerDaemonSetTemplate,
[]string{
"${OBJECT_NAMESPACE}", request.Namespace,
"${CONTAINER_IMAGE}", common.GetDiskMakerImage(),
"${RBAC_PROXY_IMAGE}", common.GetKubeRBACProxyImage(),
"${PRIORITY_CLASS_NAME}", os.Getenv("PRIORITY_CLASS_NAME"),
},
)
pairs := []string{
"${OBJECT_NAMESPACE}", request.Namespace,
"${CONTAINER_IMAGE}", common.GetDiskMakerImage(),
"${RBAC_PROXY_IMAGE}", common.GetKubeRBACProxyImage(),
"${PRIORITY_CLASS_NAME}", os.Getenv("PRIORITY_CLASS_NAME"),
"${TLS_MIN_VERSION}", tlsMinVersion,
"${TLS_CIPHER_SUITES}", tlsCipherSuites,
}

dsBytes, err := assets.ReadFileAndReplace(common.DiskMakerManagerDaemonSetTemplate, pairs)
if err != nil {
return err
}
Expand Down
31 changes: 30 additions & 1 deletion pkg/controllers/nodedaemon/reconcile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ import (
"strings"
"time"

configv1 "github.com/openshift/api/config/v1"
v1 "github.com/openshift/local-storage-operator/api/v1"
localv1alpha1 "github.com/openshift/local-storage-operator/api/v1alpha1"
"github.com/openshift/local-storage-operator/pkg/common"
"github.com/openshift/local-storage-operator/pkg/localmetrics"
lsotls "github.com/openshift/local-storage-operator/pkg/tls"
prometheusv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
Expand All @@ -20,6 +22,7 @@ import (
"k8s.io/apimachinery/pkg/types"
errorutils "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/tools/record"
"k8s.io/klog/v2"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
Expand Down Expand Up @@ -52,8 +55,10 @@ type DaemonReconciler struct {
// that reads objects from the cache and writes to the apiserver
Client client.Client
Scheme *runtime.Scheme
Recorder record.EventRecorder
deletedStaticProvisioner bool
deletedOrphanedServiceMonitor bool
lastObservedTLSConfig map[string]interface{}
}

// Reconcile reads that state of the cluster for a LocalVolumeSet object and makes changes based on the state read
Expand Down Expand Up @@ -97,7 +102,14 @@ func (r *DaemonReconciler) Reconcile(ctx context.Context, request ctrl.Request)

configMapDataHash := dataHash(configMap.Data)

diskMakerDSMutateFn := getDiskMakerDSMutateFn(request, tolerations, ownerRefs, nodeSelector, configMapDataHash)
tlsMinVersion, tlsCipherSuites, observedTLSConfig, err := lsotls.GetTLSProfileValues(ctx, r.Client, r.lastObservedTLSConfig)
if err != nil {
klog.Warningf("failed to get cluster TLS profile, proceeding with kube-rbac-proxy defaults: %v", err)
} else {
r.lastObservedTLSConfig = observedTLSConfig
}

diskMakerDSMutateFn := getDiskMakerDSMutateFn(request, tolerations, ownerRefs, nodeSelector, configMapDataHash, tlsMinVersion, tlsCipherSuites)
ds, opResult, err := CreateOrUpdateDaemonset(ctx, r.Client, diskMakerDSMutateFn)
if err != nil {
return ctrl.Result{}, err
Expand Down Expand Up @@ -240,6 +252,21 @@ func (r *DaemonReconciler) SetupWithManager(mgr ctrl.Manager) error {
return []reconcile.Request{req}
})

watchNamespace, err := common.GetWatchNamespace()
if err != nil {
return err
}

// When the cluster TLS profile changes, re-reconcile watched namespace.
enqueueAllLocalVolumeNamespaces := handler.EnqueueRequestsFromMapFunc(
func(ctx context.Context, obj client.Object) []reconcile.Request {
return []reconcile.Request{{
NamespacedName: types.NamespacedName{Namespace: watchNamespace},
}}
})

r.Recorder = mgr.GetEventRecorderFor(controllerName)

return ctrl.NewControllerManagedBy(mgr).
Named(controllerName).
For(&v1.LocalVolume{}).
Expand All @@ -249,5 +276,7 @@ func (r *DaemonReconciler) SetupWithManager(mgr ctrl.Manager) error {
// watch provisioner configmap
Watches(&corev1.ConfigMap{}, enqueueOnlyNamespace, builder.WithPredicates(common.EnqueueOnlyLabeledSubcomponents(common.ProvisionerConfigMapName))).
Watches(&v1.LocalVolume{}, enqueueOnlyNamespace).
// watch cluster TLS profile changes
Watches(&configv1.APIServer{}, enqueueAllLocalVolumeNamespaces).
Complete(r)
}
Loading