From 39b9b57b1545f53f6a9c785ad54a5c1297efb8f0 Mon Sep 17 00:00:00 2001 From: Ian Howell Date: Wed, 14 Apr 2021 14:52:20 -0500 Subject: [PATCH] Secure Redfish endpoint with BasicAuth This sets up a reverse proxy using nginx to put basic auth in front of the sushy emulator. Change-Id: Ia34195daa41793aaebbe80af3450925f514da6a8 --- config/manager/daemonset-template.yaml | 10 +++-- pkg/api/v1/vino_types.go | 8 ++++ pkg/controllers/vino_controller.go | 53 +++++++++++++++---------- pkg/controllers/vino_controller_test.go | 47 +++++++--------------- 4 files changed, 60 insertions(+), 58 deletions(-) diff --git a/config/manager/daemonset-template.yaml b/config/manager/daemonset-template.yaml index eb98809..382ca5e 100644 --- a/config/manager/daemonset-template.yaml +++ b/config/manager/daemonset-template.yaml @@ -50,17 +50,19 @@ spec: - name: etc-storage mountPath: /etc/libvirt/storage - name: sushy - ports: - - containerPort: 8000 - hostPort: 8000 image: quay.io/metal3-io/sushy-tools imagePullPolicy: IfNotPresent - command: ["/usr/local/bin/sushy-emulator"] + command: ["/usr/local/bin/sushy-emulator", "--port", "5000"] volumeMounts: - name: var-run-libvirt mountPath: /var/run/libvirt - name: var-lib-libvirt mountPath: /var/lib/libvirt + - name: vino-reverse-proxy + image: quay.io/airshipit/vino-reverse-proxy + ports: + - containerPort: 8000 + hostPort: 8000 - name: labeler image: quay.io/airshipit/nodelabeler imagePullPolicy: IfNotPresent diff --git a/pkg/api/v1/vino_types.go b/pkg/api/v1/vino_types.go index 77e257d..ca5bab9 100644 --- a/pkg/api/v1/vino_types.go +++ b/pkg/api/v1/vino_types.go @@ -39,6 +39,14 @@ const ( VinoNodeNetworkValuesAnnotation = "airshipit.org/vino.network-values" ) +// Constants for BasicAuth +const ( + EnvVarBasicAuthUsername = "BASIC_AUTH_USERNAME" + // NOTE: gosec thinks this is a hard-coded password, when in reality it + // is the name of an environment variable. We'll suppress that error + EnvVarBasicAuthPassword = "BASIC_AUTH_PASSWORD" //nolint:gosec +) + // VinoSpec defines the desired state of Vino type VinoSpec struct { // Define nodelabel parameters diff --git a/pkg/controllers/vino_controller.go b/pkg/controllers/vino_controller.go index 44039b7..3b1a5df 100644 --- a/pkg/controllers/vino_controller.go +++ b/pkg/controllers/vino_controller.go @@ -217,8 +217,10 @@ func (r *VinoReconciler) decorateDaemonSet(ctx context.Context, ds *appsv1.Daemo // TODO develop logic to derive all required ENV variables from VINO CR, and pass them // to setENV function instead if vino.Spec.VMBridge != "" { - setEnv(ctx, ds, vino) + setEnv(ctx, ds, vinov1.EnvVarVMInterfaceName, vino.Spec.VMBridge) } + setEnv(ctx, ds, vinov1.EnvVarBasicAuthUsername, vino.Spec.BMCCredentials.Username) + setEnv(ctx, ds, vinov1.EnvVarBasicAuthPassword, vino.Spec.BMCCredentials.Password) // this will help avoid colisions if we have two vino CRs in the same namespace ds.Spec.Selector.MatchLabels[vinov1.VinoLabelDSNameSelector] = vino.Name @@ -228,30 +230,37 @@ func (r *VinoReconciler) decorateDaemonSet(ctx context.Context, ds *appsv1.Daemo ds.Spec.Template.ObjectMeta.Labels[vinov1.VinoLabelDSNamespaceSelector] = vino.Namespace } -func setEnv(ctx context.Context, ds *appsv1.DaemonSet, vino *vinov1.Vino) { - for i, c := range ds.Spec.Template.Spec.Containers { - var set bool - for j, envVar := range c.Env { - if envVar.Name == vinov1.EnvVarVMInterfaceName { - logr.FromContext(ctx).Info("found env variable with vm interface name on daemonset template, overriding it", - "vino instance", vino.Namespace+"/"+vino.Name, - "container name", c.Name, - "value", envVar.Value, - ) - ds.Spec.Template.Spec.Containers[i].Env[j].Value = vino.Spec.VMBridge - set = true - break - } - } - if !set { - ds.Spec.Template.Spec.Containers[i].Env = append( - ds.Spec.Template.Spec.Containers[i].Env, corev1.EnvVar{ - Name: vinov1.EnvVarVMInterfaceName, - Value: vino.Spec.VMBridge, - }, +// setEnv iterates over all container specs and sets the variable varName to +// varValue. If varName already exists for a container, setEnv overrides it +func setEnv(ctx context.Context, ds *appsv1.DaemonSet, varName, varValue string) { + for i := range ds.Spec.Template.Spec.Containers { + setContainerEnv(ctx, &ds.Spec.Template.Spec.Containers[i], varName, varValue) + } +} + +// setContainerEnv sets the variable varName to varValue for the container. If +// varName already exists for the container, setContainerEnv overrides it +func setContainerEnv(ctx context.Context, container *corev1.Container, varName, varValue string) { + for j, envVar := range container.Env { + if envVar.Name == varName { + logr.FromContext(ctx).Info("found pre-existing environment variable on daemonset template, overriding it", + "container name", container.Name, + "environment variable", envVar.Name, + "old value", envVar.Value, + "new value", varValue, ) + container.Env[j].Value = varValue + return } } + + // If we've made it this far, the variable didn't exist. + container.Env = append( + container.Env, corev1.EnvVar{ + Name: varName, + Value: varValue, + }, + ) } func (r *VinoReconciler) waitDaemonSet(ctx context.Context, check dsWaitCondition, ds *appsv1.DaemonSet) error { diff --git a/pkg/controllers/vino_controller_test.go b/pkg/controllers/vino_controller_test.go index 1d6a4a9..9dfb67b 100644 --- a/pkg/controllers/vino_controller_test.go +++ b/pkg/controllers/vino_controller_test.go @@ -34,23 +34,13 @@ var _ = Describe("Test Setting Env variables", func() { Context("when daemonset is created", func() { l := logr.Discard() ctx := logr.NewContext(context.Background(), l) - Context("no containers defined in damonset", func() { - It("does nothing", func() { - ds := testDS() - setEnv(ctx, ds, testVINO()) - Expect(ds.Spec.Template.Spec.Containers).To(HaveLen(0)) - }) - - }) Context("when daemonset has containers", func() { - It("sets env interface variable to every container", func() { - vino := testVINO() + It("sets env variable to every container", func() { ifName := "eth0" - vino.Spec.VMBridge = ifName ds := testDS() ds.Spec.Template.Spec.Containers = make([]corev1.Container, 3) - setEnv(ctx, ds, vino) + setEnv(ctx, ds, vinov1.EnvVarVMInterfaceName, ifName) for _, container := range ds.Spec.Template.Spec.Containers { Expect(container.Env).To(HaveLen(1)) @@ -60,24 +50,22 @@ var _ = Describe("Test Setting Env variables", func() { }) }) - Context("when daemonset has container with wrong env var values", func() { + Context("when daemonset has container with pre-existing env var values", func() { It("overrides that variable in the container", func() { - vino := testVINO() ifName := "eth0" - vino.Spec.VMBridge = ifName ds := testDS() ds.Spec.Template.Spec.Containers = []corev1.Container{ { Env: []corev1.EnvVar{ { Name: vinov1.EnvVarVMInterfaceName, - Value: "wrong-value", + Value: "old-value", }, }, }, } - setEnv(ctx, ds, vino) + setEnv(ctx, ds, vinov1.EnvVarVMInterfaceName, ifName) Expect(ds.Spec.Template.Spec.Containers).To(HaveLen(1)) container := ds.Spec.Template.Spec.Containers[0] Expect(container.Env).To(HaveLen(1)) @@ -86,21 +74,19 @@ var _ = Describe("Test Setting Env variables", func() { }) }) Context("when daemonset containers don't have required variable", func() { - It("overrides that variable in the container", func() { - vino := testVINO() + It("adds that variable to all the containers", func() { ifName := "eth0" - vino.Spec.VMBridge = ifName ds := testDS() ds.Spec.Template.Spec.Containers = []corev1.Container{ { Env: []corev1.EnvVar{ { Name: "bar", - Value: "wrong-value", + Value: "old-value", }, { Name: "foo", - Value: "wrong-value", + Value: "old-value", }, }, }, @@ -108,17 +94,17 @@ var _ = Describe("Test Setting Env variables", func() { Env: []corev1.EnvVar{ { Name: "foo", - Value: "wrong-value", + Value: "old-value", }, { Name: "bar", - Value: "wrong-value", + Value: "old-value", }, }, }, } - setEnv(ctx, ds, vino) + setEnv(ctx, ds, vinov1.EnvVarVMInterfaceName, ifName) Expect(ds.Spec.Template.Spec.Containers).To(HaveLen(2)) for _, container := range ds.Spec.Template.Spec.Containers { @@ -139,31 +125,28 @@ var _ = Describe("Test Setting Env variables", func() { }) Context("when daemonset container has many variables", func() { It("it sets required variable only single time", func() { - vino := testVINO() ifName := "eth0" - vino.Spec.VMBridge = ifName ds := testDS() ds.Spec.Template.Spec.Containers = []corev1.Container{ { Env: []corev1.EnvVar{ { Name: "foo", - Value: "wrong-value", + Value: "old-value", }, { Name: vinov1.EnvVarVMInterfaceName, - Value: "wrong-value", + Value: "old-value", }, - { Name: "bar", - Value: "wrong-value", + Value: "old-value", }, }, }, } - setEnv(ctx, ds, vino) + setEnv(ctx, ds, vinov1.EnvVarVMInterfaceName, ifName) Expect(ds.Spec.Template.Spec.Containers).To(HaveLen(1)) container := ds.Spec.Template.Spec.Containers[0] Expect(container.Env).To(HaveLen(3))