-
Notifications
You must be signed in to change notification settings - Fork 259
CNTRLPLANE-2241: kms: add helper for TechPreview V1 #2090
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| package kms | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| "github.com/openshift/api/features" | ||
| "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" | ||
| corev1 "k8s.io/api/core/v1" | ||
| ) | ||
|
|
||
| // AddKMSPluginVolumeAndMountToPodSpec conditionally adds the KMS plugin volume mount to the specified container. | ||
| // It assumes the pod spec does not already contain the KMS volume or mount; no deduplication is performed. | ||
| // Deprecated: this is a temporary solution to get KMS TP v1 out. We should come up with a different approach afterwards. | ||
| func AddKMSPluginVolumeAndMountToPodSpec(podSpec *corev1.PodSpec, containerName string, featureGateAccessor featuregates.FeatureGateAccess) error { | ||
| if podSpec == nil { | ||
| return fmt.Errorf("pod spec cannot be nil") | ||
| } | ||
|
|
||
| if !featureGateAccessor.AreInitialFeatureGatesObserved() { | ||
| return nil | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think this is what we usually return when FG hasn't been observed, right ?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, in that case it's not possible to make a decision |
||
| } | ||
|
|
||
| featureGates, err := featureGateAccessor.CurrentFeatureGates() | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get feature gates: %w", err) | ||
| } | ||
|
|
||
| if !featureGates.Enabled(features.FeatureGateKMSEncryption) { | ||
| return nil | ||
| } | ||
|
|
||
bertinatto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| containerIndex := -1 | ||
| for i, container := range podSpec.Containers { | ||
| if container.Name == containerName { | ||
| containerIndex = i | ||
| break | ||
| } | ||
| } | ||
|
|
||
| if containerIndex < 0 { | ||
| return fmt.Errorf("container %s not found", containerName) | ||
| } | ||
|
|
||
| container := &podSpec.Containers[containerIndex] | ||
| container.VolumeMounts = append(container.VolumeMounts, | ||
| corev1.VolumeMount{ | ||
| Name: "kms-plugin-socket", | ||
| MountPath: "/var/run/kmsplugin", | ||
| }, | ||
| ) | ||
|
|
||
| directoryOrCreate := corev1.HostPathDirectoryOrCreate | ||
| podSpec.Volumes = append(podSpec.Volumes, | ||
| corev1.Volume{ | ||
| Name: "kms-plugin-socket", | ||
| VolumeSource: corev1.VolumeSource{ | ||
| HostPath: &corev1.HostPathVolumeSource{ | ||
| Path: "/var/run/kmsplugin", | ||
| Type: &directoryOrCreate, | ||
| }, | ||
| }, | ||
| }, | ||
| ) | ||
|
|
||
| return nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,197 @@ | ||
| package kms | ||
|
|
||
| import ( | ||
| "errors" | ||
| "testing" | ||
|
|
||
| configv1 "github.com/openshift/api/config/v1" | ||
| "github.com/openshift/api/features" | ||
| "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" | ||
| corev1 "k8s.io/api/core/v1" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestAddKMSPluginVolume(t *testing.T) { | ||
| directoryOrCreate := corev1.HostPathDirectoryOrCreate | ||
|
|
||
| tests := []struct { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a personal preference. I really like it when test definitions include inputs and expected outputs. It allows me to review test cases without looking at the logic. If you feel the same way, we could slightly update the unit test to include
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ack; included |
||
| name string | ||
| featureGateAccessor featuregates.FeatureGateAccess | ||
| actualPodSpec *corev1.PodSpec | ||
| expectedPodSpec *corev1.PodSpec | ||
| containerName string | ||
| expectError bool | ||
| }{ | ||
| { | ||
| name: "nil pod: returns error", | ||
| featureGateAccessor: featuregates.NewHardcodedFeatureGateAccess( | ||
| []configv1.FeatureGateName{features.FeatureGateKMSEncryption}, | ||
| []configv1.FeatureGateName{}, | ||
| ), | ||
| actualPodSpec: nil, | ||
| expectedPodSpec: nil, | ||
| containerName: "kube-apiserver", | ||
| expectError: true, | ||
| }, | ||
| { | ||
| name: "container not found: returns error", | ||
| featureGateAccessor: featuregates.NewHardcodedFeatureGateAccess( | ||
| []configv1.FeatureGateName{features.FeatureGateKMSEncryption}, | ||
| []configv1.FeatureGateName{}, | ||
| ), | ||
| actualPodSpec: &corev1.PodSpec{ | ||
| Containers: []corev1.Container{ | ||
| {Name: "other-container"}, | ||
| }, | ||
| }, | ||
| expectedPodSpec: nil, | ||
| containerName: "kube-apiserver", | ||
| expectError: true, | ||
| }, | ||
| { | ||
| name: "feature gates not observed: no volume added", | ||
| featureGateAccessor: featuregates.NewHardcodedFeatureGateAccessForTesting( | ||
| nil, | ||
| nil, | ||
| make(chan struct{}), | ||
| nil, | ||
| ), | ||
| actualPodSpec: &corev1.PodSpec{ | ||
| Containers: []corev1.Container{ | ||
| {Name: "kube-apiserver"}, | ||
| }, | ||
| }, | ||
| expectedPodSpec: &corev1.PodSpec{ | ||
| Containers: []corev1.Container{ | ||
| {Name: "kube-apiserver"}, | ||
| }, | ||
| }, | ||
| containerName: "kube-apiserver", | ||
| expectError: false, | ||
| }, | ||
| { | ||
| name: "feature gate accessor error: returns error", | ||
| featureGateAccessor: featuregates.NewHardcodedFeatureGateAccessForTesting( | ||
| nil, | ||
| nil, | ||
| func() chan struct{} { ch := make(chan struct{}); close(ch); return ch }(), | ||
| errors.New("some error"), | ||
| ), | ||
| actualPodSpec: &corev1.PodSpec{ | ||
| Containers: []corev1.Container{ | ||
| {Name: "kube-apiserver"}, | ||
| }, | ||
| }, | ||
| expectedPodSpec: nil, | ||
| containerName: "kube-apiserver", | ||
| expectError: true, | ||
| }, | ||
| { | ||
| name: "KMSEncryption feature gate disabled: no volume added", | ||
| featureGateAccessor: featuregates.NewHardcodedFeatureGateAccess( | ||
| []configv1.FeatureGateName{}, | ||
| []configv1.FeatureGateName{features.FeatureGateKMSEncryption}, | ||
| ), | ||
| actualPodSpec: &corev1.PodSpec{ | ||
| Containers: []corev1.Container{ | ||
| {Name: "kube-apiserver"}, | ||
| }, | ||
| }, | ||
| expectedPodSpec: &corev1.PodSpec{ | ||
| Containers: []corev1.Container{ | ||
| {Name: "kube-apiserver"}, | ||
| }, | ||
| }, | ||
| containerName: "kube-apiserver", | ||
| expectError: false, | ||
| }, | ||
| { | ||
| name: "KMSEncryption feature gate enabled: volume and mount added", | ||
| featureGateAccessor: featuregates.NewHardcodedFeatureGateAccess( | ||
| []configv1.FeatureGateName{features.FeatureGateKMSEncryption}, | ||
| []configv1.FeatureGateName{}, | ||
| ), | ||
| actualPodSpec: &corev1.PodSpec{ | ||
| Containers: []corev1.Container{ | ||
| {Name: "kube-apiserver"}, | ||
| }, | ||
| }, | ||
| expectedPodSpec: &corev1.PodSpec{ | ||
| Containers: []corev1.Container{ | ||
| { | ||
| Name: "kube-apiserver", | ||
| VolumeMounts: []corev1.VolumeMount{ | ||
| {Name: "kms-plugin-socket", MountPath: "/var/run/kmsplugin"}, | ||
| }, | ||
| }, | ||
| }, | ||
| Volumes: []corev1.Volume{ | ||
| { | ||
| Name: "kms-plugin-socket", | ||
| VolumeSource: corev1.VolumeSource{ | ||
| HostPath: &corev1.HostPathVolumeSource{ | ||
| Path: "/var/run/kmsplugin", | ||
| Type: &directoryOrCreate, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| containerName: "kube-apiserver", | ||
| expectError: false, | ||
| }, | ||
| { | ||
| name: "KMSEncryption feature gate enabled: only kube-apiserver container gets mount", | ||
| featureGateAccessor: featuregates.NewHardcodedFeatureGateAccess( | ||
| []configv1.FeatureGateName{features.FeatureGateKMSEncryption}, | ||
| []configv1.FeatureGateName{}, | ||
| ), | ||
| actualPodSpec: &corev1.PodSpec{ | ||
| Containers: []corev1.Container{ | ||
| {Name: "other-container"}, | ||
| {Name: "kube-apiserver"}, | ||
| {Name: "another-container"}, | ||
| }, | ||
| }, | ||
| expectedPodSpec: &corev1.PodSpec{ | ||
| Containers: []corev1.Container{ | ||
| {Name: "other-container"}, | ||
| { | ||
| Name: "kube-apiserver", | ||
| VolumeMounts: []corev1.VolumeMount{ | ||
| {Name: "kms-plugin-socket", MountPath: "/var/run/kmsplugin"}, | ||
| }, | ||
| }, | ||
| {Name: "another-container"}, | ||
| }, | ||
| Volumes: []corev1.Volume{ | ||
| { | ||
| Name: "kms-plugin-socket", | ||
| VolumeSource: corev1.VolumeSource{ | ||
| HostPath: &corev1.HostPathVolumeSource{ | ||
| Path: "/var/run/kmsplugin", | ||
| Type: &directoryOrCreate, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| containerName: "kube-apiserver", | ||
| expectError: false, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| err := AddKMSPluginVolumeAndMountToPodSpec(tt.actualPodSpec, tt.containerName, tt.featureGateAccessor) | ||
|
|
||
| if tt.expectError { | ||
| require.Error(t, err) | ||
| return | ||
| } | ||
| require.NoError(t, err) | ||
| require.Equal(t, tt.expectedPodSpec, tt.actualPodSpec) | ||
| }) | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.