From 435a7f66953c24096a50e682c59a79dd60d49e7e Mon Sep 17 00:00:00 2001 From: Ethan Mosbaugh Date: Tue, 22 Oct 2024 11:23:36 -0700 Subject: [PATCH] fix: persist http proxy settings and security context on upgrades --- cmd/kots/cli/admin-console-upgrade.go | 16 +- pkg/kotsadm/api.go | 135 +++++++++------- pkg/kotsadm/api_test.go | 222 ++++++++++++++++++++++++++ pkg/kotsadm/main.go | 19 ++- pkg/kotsadm/secrets.go | 4 +- pkg/kotsadm/types/upgradeoptions.go | 2 +- 6 files changed, 330 insertions(+), 68 deletions(-) create mode 100644 pkg/kotsadm/api_test.go diff --git a/cmd/kots/cli/admin-console-upgrade.go b/cmd/kots/cli/admin-console-upgrade.go index 7af772e040..a1e327afdb 100644 --- a/cmd/kots/cli/admin-console-upgrade.go +++ b/cmd/kots/cli/admin-console-upgrade.go @@ -72,12 +72,11 @@ func AdminConsoleUpgradeCmd() *cobra.Command { simultaneousUploads, _ := strconv.Atoi(v.GetString("airgap-upload-parallelism")) upgradeOptions := kotsadmtypes.UpgradeOptions{ - Namespace: namespace, - ForceUpgradeKurl: v.GetBool("force-upgrade-kurl"), - EnsureRBAC: v.GetBool("ensure-rbac"), - SimultaneousUploads: simultaneousUploads, - IncludeMinio: includeMinio, - StrictSecurityContext: v.GetBool("strict-security-context"), + Namespace: namespace, + ForceUpgradeKurl: v.GetBool("force-upgrade-kurl"), + EnsureRBAC: v.GetBool("ensure-rbac"), + SimultaneousUploads: simultaneousUploads, + IncludeMinio: includeMinio, RegistryConfig: kotsadmtypes.RegistryConfig{ OverrideVersion: v.GetString("kotsadm-tag"), @@ -88,6 +87,11 @@ func AdminConsoleUpgradeCmd() *cobra.Command { }, } + if v.IsSet("strict-security-context") { + ok := v.GetBool("strict-security-context") + upgradeOptions.StrictSecurityContext = &ok + } + timeout, err := time.ParseDuration(v.GetString("wait-duration")) if err != nil { return errors.Wrap(err, "failed to parse timeout value") diff --git a/pkg/kotsadm/api.go b/pkg/kotsadm/api.go index 61f8fee82e..142b98ac15 100644 --- a/pkg/kotsadm/api.go +++ b/pkg/kotsadm/api.go @@ -79,43 +79,20 @@ func removeNodeAPIClusterRBAC(deployOptions *types.DeployOptions, clientset *kub return nil } -func getAPIAutoCreateClusterToken(namespace string, clientset *kubernetes.Clientset) (string, error) { - autoCreateClusterTokenSecretVal, err := getAPIClusterToken(namespace, clientset) +func getAPIAutoCreateClusterToken(namespace string, cli kubernetes.Interface) (string, error) { + autoCreateClusterTokenSecretVal, err := getAPIClusterToken(namespace, cli) if err != nil { return "", errors.Wrap(err, "failed to get autocreate cluster token from secret") } else if autoCreateClusterTokenSecretVal != "" { return autoCreateClusterTokenSecretVal, nil } - var containers []corev1.Container - - existingDeployment, err := clientset.AppsV1().Deployments(namespace).Get(context.TODO(), "kotsadm", metav1.GetOptions{}) - if err != nil && !kuberneteserrors.IsNotFound(err) { - return "", errors.Wrap(err, "failed to read deployment") - } - if err == nil { - containers = existingDeployment.Spec.Template.Spec.Containers - } else { - // deployment not found, check if there's a statefulset - existingStatefulSet, err := clientset.AppsV1().StatefulSets(namespace).Get(context.TODO(), "kotsadm", metav1.GetOptions{}) - if err != nil { - return "", errors.Wrap(err, "failed to read statefulset") - } - containers = existingStatefulSet.Spec.Template.Spec.Containers - } - - containerIdx := -1 - for idx, c := range containers { - if c.Name == "kotsadm" { - containerIdx = idx - } - } - - if containerIdx == -1 { - return "", errors.New("failed to find kotsadm container in statefulset") + container, err := getKotsadmContainer(namespace, cli) + if err != nil { + return "", errors.Wrap(err, "failed to get kotsadm container") } - for _, env := range containers[containerIdx].Env { + for _, env := range container.Env { if env.Name == "AUTO_CREATE_CLUSTER_TOKEN" { return env.Value, nil } @@ -124,8 +101,8 @@ func getAPIAutoCreateClusterToken(namespace string, clientset *kubernetes.Client return "", errors.New("failed to find autocreateclustertoken env on api statefulset") } -func getKotsInstallID(namespace string, clientset *kubernetes.Clientset) (string, error) { - configMap, err := clientset.CoreV1().ConfigMaps(namespace).Get(context.TODO(), types.KotsadmConfigMap, metav1.GetOptions{}) +func getKotsInstallID(namespace string, cli kubernetes.Interface) (string, error) { + configMap, err := cli.CoreV1().ConfigMaps(namespace).Get(context.TODO(), types.KotsadmConfigMap, metav1.GetOptions{}) if err != nil && !kuberneteserrors.IsNotFound(err) { return "", errors.Wrap(err, "failed to read configmap") } @@ -137,35 +114,12 @@ func getKotsInstallID(namespace string, clientset *kubernetes.Clientset) (string // configmap does not exist or does not have the installation id, check deployment or statefulset - var containers []corev1.Container - - existingDeployment, err := clientset.AppsV1().Deployments(namespace).Get(context.TODO(), "kotsadm", metav1.GetOptions{}) - if err != nil && !kuberneteserrors.IsNotFound(err) { - return "", errors.Wrap(err, "failed to get deployment") - } - if err == nil { - containers = existingDeployment.Spec.Template.Spec.Containers - } else { - // deployment not found, check if there's a statefulset - existingStatefulSet, err := clientset.AppsV1().StatefulSets(namespace).Get(context.TODO(), "kotsadm", metav1.GetOptions{}) - if err != nil { - return "", errors.Wrap(err, "failed to get statefulset") - } - containers = existingStatefulSet.Spec.Template.Spec.Containers - } - - containerIdx := -1 - for idx, c := range containers { - if c.Name == "kotsadm" { - containerIdx = idx - } - } - - if containerIdx == -1 { - return "", errors.New("failed to find kotsadm container") + container, err := getKotsadmContainer(namespace, cli) + if err != nil { + return "", errors.Wrap(err, "failed to get kotsadm container") } - for _, env := range containers[containerIdx].Env { + for _, env := range container.Env { if env.Name == "KOTS_INSTALL_ID" { return env.Value, nil } @@ -176,3 +130,68 @@ func getKotsInstallID(namespace string, clientset *kubernetes.Clientset) (string // - they were affected by a bug that removed the env var on upgrade return "", nil } + +func getHTTPProxySettings(namespace string, cli kubernetes.Interface) (httpProxy, httpsProxy, noProxy string, err error) { + container, err := getKotsadmContainer(namespace, cli) + if err != nil { + return "", "", "", errors.Wrap(err, "failed to get kotsadm container") + } + + for _, env := range container.Env { + if env.Name == "HTTP_PROXY" { + httpProxy = env.Value + } + if env.Name == "HTTPS_PROXY" { + httpsProxy = env.Value + } + if env.Name == "NO_PROXY" { + noProxy = env.Value + } + } + + return httpProxy, httpsProxy, noProxy, nil +} + +func hasStrictSecurityContext(namespace string, cli kubernetes.Interface) (bool, error) { + podSpec, err := getKotsadmPodSpec(namespace, cli) + if err != nil { + return false, errors.Wrap(err, "failed to get kotsadm pod spec") + } + + if podSpec.SecurityContext == nil { + return false, nil + } + if podSpec.SecurityContext.RunAsNonRoot == nil { + return false, nil + } + + return *podSpec.SecurityContext.RunAsNonRoot, nil +} + +func getKotsadmPodSpec(namespace string, cli kubernetes.Interface) (*corev1.PodSpec, error) { + deploy, err := cli.AppsV1().Deployments(namespace).Get(context.TODO(), "kotsadm", metav1.GetOptions{}) + if err == nil { + return &deploy.Spec.Template.Spec, nil + } else if !kuberneteserrors.IsNotFound(err) { + return nil, errors.Wrap(err, "failed to get deployment") + } + + sts, err := cli.AppsV1().StatefulSets(namespace).Get(context.TODO(), "kotsadm", metav1.GetOptions{}) + if err != nil { + return nil, errors.Wrap(err, "failed to get statefulset") + } + return &sts.Spec.Template.Spec, nil +} + +func getKotsadmContainer(namespace string, cli kubernetes.Interface) (*corev1.Container, error) { + podSpec, err := getKotsadmPodSpec(namespace, cli) + if err != nil { + return nil, errors.Wrap(err, "failed to get kotsadm pod spec") + } + for _, c := range podSpec.Containers { + if c.Name == "kotsadm" { + return &c, nil + } + } + return nil, errors.New("failed to find kotsadm container") +} diff --git a/pkg/kotsadm/api_test.go b/pkg/kotsadm/api_test.go new file mode 100644 index 0000000000..e7d6eb78f0 --- /dev/null +++ b/pkg/kotsadm/api_test.go @@ -0,0 +1,222 @@ +package kotsadm + +import ( + "testing" + + "github.com/aws/smithy-go/ptr" + "gopkg.in/go-playground/assert.v1" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" +) + +func Test_getHTTPProxySettings(t *testing.T) { + tests := []struct { + name string + objects []runtime.Object + wantHttpProxy string + wantHttpsProxy string + wantNoProxy string + wantErr bool + }{ + { + name: "found in deployment", + objects: []runtime.Object{ + &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kotsadm", + Namespace: "kotsadm", + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "kotsadm", + Env: []corev1.EnvVar{ + { + Name: "HTTP_PROXY", + Value: "http://proxy.com", + }, + { + Name: "HTTPS_PROXY", + Value: "https://proxy.com", + }, + { + Name: "NO_PROXY", + Value: "localhost", + }, + }, + }, + }, + }, + }, + }, + }, + }, + wantHttpProxy: "http://proxy.com", + wantHttpsProxy: "https://proxy.com", + wantNoProxy: "localhost", + wantErr: false, + }, + { + name: "found in statefulset", + objects: []runtime.Object{ + &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kotsadm", + Namespace: "kotsadm", + }, + Spec: appsv1.StatefulSetSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "kotsadm", + Env: []corev1.EnvVar{ + { + Name: "HTTP_PROXY", + Value: "http://proxy.com", + }, + { + Name: "HTTPS_PROXY", + Value: "https://proxy.com", + }, + { + Name: "NO_PROXY", + Value: "localhost", + }, + }, + }, + }, + }, + }, + }, + }, + }, + wantHttpProxy: "http://proxy.com", + wantHttpsProxy: "https://proxy.com", + wantNoProxy: "localhost", + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cli := fake.NewSimpleClientset(tt.objects...) + + gotHttpProxy, gotHttpsProxy, gotNoProxy, err := getHTTPProxySettings("kotsadm", cli) + if (err != nil) != tt.wantErr { + t.Errorf("getHTTPProxySettings() error = %v, wantErr %v", err, tt.wantErr) + return + } + + assert.Equal(t, gotHttpProxy, tt.wantHttpProxy) + assert.Equal(t, gotHttpsProxy, tt.wantHttpsProxy) + assert.Equal(t, gotNoProxy, tt.wantNoProxy) + }) + } +} + +func Test_hasStrictSecurityContext(t *testing.T) { + tests := []struct { + name string + objects []runtime.Object + want bool + wantErr bool + }{ + { + name: "strict", + objects: []runtime.Object{ + &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kotsadm", + Namespace: "kotsadm", + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "kotsadm", + }, + }, + SecurityContext: &corev1.PodSecurityContext{ + RunAsUser: ptr.Int64(1001), + RunAsNonRoot: ptr.Bool(true), + }, + }, + }, + }, + }, + }, + want: true, + wantErr: false, + }, + { + name: "not strict", + objects: []runtime.Object{ + &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kotsadm", + Namespace: "kotsadm", + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "kotsadm", + }, + }, + SecurityContext: &corev1.PodSecurityContext{ + RunAsUser: ptr.Int64(1001), + }, + }, + }, + }, + }, + }, + want: false, + wantErr: false, + }, + { + name: "nil", + objects: []runtime.Object{ + &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kotsadm", + Namespace: "kotsadm", + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "kotsadm", + }, + }, + }, + }, + }, + }, + }, + want: false, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cli := fake.NewSimpleClientset(tt.objects...) + + got, err := hasStrictSecurityContext("kotsadm", cli) + if (err != nil) != tt.wantErr { + t.Errorf("hasStrictSecurityContext() error = %v, wantErr %v", err, tt.wantErr) + return + } + + assert.Equal(t, got, tt.want) + }) + } +} diff --git a/pkg/kotsadm/main.go b/pkg/kotsadm/main.go index 7ddeb1a42c..b9b2cdacad 100644 --- a/pkg/kotsadm/main.go +++ b/pkg/kotsadm/main.go @@ -124,7 +124,10 @@ func Upgrade(clientset *kubernetes.Clientset, upgradeOptions types.UpgradeOption deployOptions.EnsureRBAC = upgradeOptions.EnsureRBAC deployOptions.SimultaneousUploads = upgradeOptions.SimultaneousUploads deployOptions.IncludeMinio = upgradeOptions.IncludeMinio - deployOptions.StrictSecurityContext = upgradeOptions.StrictSecurityContext + // Override with value set by user + if upgradeOptions.StrictSecurityContext != nil { + deployOptions.StrictSecurityContext = *upgradeOptions.StrictSecurityContext + } if deployOptions.IncludeMinio { deployOptions.MigrateToMinioXl, deployOptions.CurrentMinioImage, err = IsMinioXlMigrationNeeded(clientset, deployOptions.Namespace) @@ -1118,6 +1121,20 @@ func ReadDeployOptionsFromCluster(namespace string, clientset *kubernetes.Client deployOptions.IngressConfig = *ingressConfig } + httpProxy, httpsProxy, noProxy, err := getHTTPProxySettings(deployOptions.Namespace, clientset) + if err != nil { + return nil, errors.Wrap(err, "failed to get http proxy settings") + } + deployOptions.HTTPProxyEnvValue = httpProxy + deployOptions.HTTPSProxyEnvValue = httpsProxy + deployOptions.NoProxyEnvValue = noProxy + + strictSecurityContext, err := hasStrictSecurityContext(deployOptions.Namespace, clientset) + if err != nil { + return nil, errors.Wrap(err, "failed to check if strict security context is enabled") + } + deployOptions.StrictSecurityContext = strictSecurityContext + return &deployOptions, nil } diff --git a/pkg/kotsadm/secrets.go b/pkg/kotsadm/secrets.go index 49134a6779..c0080d1885 100644 --- a/pkg/kotsadm/secrets.go +++ b/pkg/kotsadm/secrets.go @@ -314,8 +314,8 @@ func ensureAPIClusterTokenSecret(deployOptions types.DeployOptions, clientset *k return nil } -func getAPIClusterToken(namespace string, clientset *kubernetes.Clientset) (string, error) { - apiSecret, err := clientset.CoreV1().Secrets(namespace).Get(context.TODO(), types.ClusterTokenSecret, metav1.GetOptions{}) +func getAPIClusterToken(namespace string, cli kubernetes.Interface) (string, error) { + apiSecret, err := cli.CoreV1().Secrets(namespace).Get(context.TODO(), types.ClusterTokenSecret, metav1.GetOptions{}) if err != nil { if kuberneteserrors.IsNotFound(err) { return "", nil diff --git a/pkg/kotsadm/types/upgradeoptions.go b/pkg/kotsadm/types/upgradeoptions.go index 6256ef4765..8b39de5213 100644 --- a/pkg/kotsadm/types/upgradeoptions.go +++ b/pkg/kotsadm/types/upgradeoptions.go @@ -9,7 +9,7 @@ type UpgradeOptions struct { ForceUpgradeKurl bool Timeout time.Duration EnsureRBAC bool - StrictSecurityContext bool + StrictSecurityContext *bool SimultaneousUploads int IncludeMinio bool