From a47edbd60430d713fedfefaba54f866d229abbeb Mon Sep 17 00:00:00 2001 From: Praveen Rewar <8457124+praveenrewar@users.noreply.github.com> Date: Sun, 24 Sep 2023 20:57:10 +0530 Subject: [PATCH] Fix app-namespace usage for cluster options During the introduction of defaultNamespace feature, we started using --app-namespace flag from kapp which should be used carefully when using cluster options instead of service account Signed-off-by: Praveen Rewar <8457124+praveenrewar@users.noreply.github.com> --- pkg/deploy/kapp.go | 9 ++++++--- pkg/kubeconfig/kubeconfig.go | 14 +++++++++++--- pkg/kubeconfig/kubeconfig_restricted.go | 8 +++++++- pkg/kubeconfig/kubeconfig_secrets.go | 6 ++++-- pkg/kubeconfig/service_accounts.go | 7 ++++--- 5 files changed, 32 insertions(+), 12 deletions(-) diff --git a/pkg/deploy/kapp.go b/pkg/deploy/kapp.go index 5618bab79b..49d599521c 100644 --- a/pkg/deploy/kapp.go +++ b/pkg/deploy/kapp.go @@ -63,7 +63,7 @@ func (a *Kapp) Deploy(tplOutput string, startedApplyingFunc func(), metadataFile := filepath.Join(tmpMetadataDir.Path(), "app-metadata.yml") - args, err := a.addDeployArgs([]string{"deploy", "--app-metadata-file-output", metadataFile, "--prev-app", a.oldManagedName(), "-f", "-", "--app-namespace", a.appNamespace}) + args, err := a.addDeployArgs([]string{"deploy", "--app-metadata-file-output", metadataFile, "--prev-app", a.oldManagedName(), "-f", "-"}) if err != nil { return exec.NewCmdRunResultWithErr(err) } @@ -90,7 +90,7 @@ func (a *Kapp) Deploy(tplOutput string, startedApplyingFunc func(), // Delete takes the app name, it shells out, running kapp delete ... func (a *Kapp) Delete(startedApplyingFunc func(), changedFunc func(exec.CmdRunResult)) exec.CmdRunResult { - args, err := a.addDeleteArgs([]string{"delete", "--prev-app", a.oldManagedName(), "--app-namespace", a.appNamespace}) + args, err := a.addDeleteArgs([]string{"delete", "--prev-app", a.oldManagedName()}) if err != nil { return exec.NewCmdRunResultWithErr(err) } @@ -120,7 +120,6 @@ func (a *Kapp) Inspect() exec.CmdRunResult { // TODO is there a better way to deal with this? "--filter", `{"not":{"resource":{"kinds":["PodMetrics"]}}}`, "--tty", - "--app-namespace", a.appNamespace, }) if err != nil { return exec.NewCmdRunResultWithErr(err) @@ -260,6 +259,10 @@ func (a *Kapp) addGenericArgs(args []string, appName string) ([]string, []string args = append(args, []string{"--namespace", a.clusterAccess.Namespace}...) } + if len(a.clusterAccess.DeployNamespace) > 0 { + args = append(args, []string{"--app-namespace", a.clusterAccess.DeployNamespace}...) + } + switch { case a.clusterAccess.Kubeconfig != nil: env = append(env, "KAPP_KUBECONFIG_YAML="+a.clusterAccess.Kubeconfig.AsYAML()) diff --git a/pkg/kubeconfig/kubeconfig.go b/pkg/kubeconfig/kubeconfig.go index 7fbf5e6ea1..ae5db0a190 100644 --- a/pkg/kubeconfig/kubeconfig.go +++ b/pkg/kubeconfig/kubeconfig.go @@ -32,6 +32,8 @@ type AccessInfo struct { Name string Namespace string + DeployNamespace string + Kubeconfig *Restricted DangerousUsePodServiceAccount bool } @@ -47,7 +49,7 @@ func NewKubeconfig(coreClient kubernetes.Interface, log logr.Logger) *Kubeconfig // ClusterAccess takes cluster info and a ServiceAccount Name, and returns a populated kubeconfig that can connect to a cluster. // if the saName is empty then you'll connect to a cluster via the clusterOpts inside the genericOpts, otherwise you'll use the specified SA. -func (k Kubeconfig) ClusterAccess(saName string, clusterOpts *v1alpha1.AppCluster, accessLocation AccessLocation, preferredNamespace string) (AccessInfo, error) { +func (k Kubeconfig) ClusterAccess(saName string, clusterOpts *v1alpha1.AppCluster, accessLocation AccessLocation, defaultNamespace string) (AccessInfo, error) { var err error var clusterAccessInfo AccessInfo @@ -63,13 +65,19 @@ func (k Kubeconfig) ClusterAccess(saName string, clusterOpts *v1alpha1.AppCluste if err != nil { return AccessInfo{}, err } + // Use kubeconfig preferred namespace as deploy namespace + // if defaultNamespace is provided and cluster.namespace is not provided, + if defaultNamespace != "" && clusterAccessInfo.DeployNamespace == "" { + clusterAccessInfo.DeployNamespace = clusterAccessInfo.Kubeconfig.preferredNamespace + } default: return AccessInfo{}, fmt.Errorf("Expected service account or cluster specified") } - // If preferredNamespace is "", then kubeconfig preferred namespace will be used - clusterAccessInfo.Namespace = preferredNamespace + if defaultNamespace != "" { + clusterAccessInfo.Namespace = defaultNamespace + } return clusterAccessInfo, nil } diff --git a/pkg/kubeconfig/kubeconfig_restricted.go b/pkg/kubeconfig/kubeconfig_restricted.go index 8f11e0d690..69fb94cbdb 100644 --- a/pkg/kubeconfig/kubeconfig_restricted.go +++ b/pkg/kubeconfig/kubeconfig_restricted.go @@ -13,6 +13,8 @@ import ( // Restricted contains a kubernetes kubeconfig as a string type Restricted struct { result string + + preferredNamespace string } // NewKubeconfigRestricted takes kubeconfig yaml as input and returns kubeconfig yaml with certain fields restricted (removed). @@ -65,6 +67,7 @@ func NewKubeconfigRestricted(input string) (*Restricted, error) { }) } + preferredNamespace := "" for _, inputCtx := range inputConfig.Contexts { resultConfig.Contexts = append(resultConfig.Contexts, clientcmd.NamedContext{ Name: inputCtx.Name, @@ -74,6 +77,9 @@ func NewKubeconfigRestricted(input string) (*Restricted, error) { Namespace: inputCtx.Context.Namespace, }, }) + if inputCtx.Name == inputConfig.CurrentContext { + preferredNamespace = inputCtx.Context.Namespace + } } bs, err := yaml.Marshal(resultConfig) @@ -81,7 +87,7 @@ func NewKubeconfigRestricted(input string) (*Restricted, error) { return nil, fmt.Errorf("Marshaling kubeconfig: %s", err) } - return &Restricted{string(bs)}, nil + return &Restricted{string(bs), preferredNamespace}, nil } // AsYAML returns a string formatted kubernetes kubeconfig diff --git a/pkg/kubeconfig/kubeconfig_secrets.go b/pkg/kubeconfig/kubeconfig_secrets.go index d0fb2c1bf0..36ea370523 100644 --- a/pkg/kubeconfig/kubeconfig_secrets.go +++ b/pkg/kubeconfig/kubeconfig_secrets.go @@ -49,8 +49,10 @@ func (s *Secrets) Find(accessLocation AccessLocation, Name: accessLocation.Name, // Override destination namespace; if it's empty // assume kubeconfig contains preferred namespace - Namespace: clusterOpts.Namespace, - Kubeconfig: kubeconfigRestricted, + Namespace: clusterOpts.Namespace, + // Use provided namespace as app namespace + DeployNamespace: clusterOpts.Namespace, + Kubeconfig: kubeconfigRestricted, } return pgoForCluster, nil diff --git a/pkg/kubeconfig/service_accounts.go b/pkg/kubeconfig/service_accounts.go index 7b089e877a..fde14ca9ad 100644 --- a/pkg/kubeconfig/service_accounts.go +++ b/pkg/kubeconfig/service_accounts.go @@ -51,9 +51,10 @@ func (s *ServiceAccounts) Find(accessLocation AccessLocation, saName string) (Ac } pgoForSA := AccessInfo{ - Name: accessLocation.Name, - Namespace: "", // Assume kubeconfig contains preferred namespace from SA - Kubeconfig: kubeconfigRestricted, + Name: accessLocation.Name, + Namespace: "", // Assume kubeconfig contains preferred namespace from SA + DeployNamespace: accessLocation.Namespace, // App namespace is same as SA namespace + Kubeconfig: kubeconfigRestricted, } return pgoForSA, nil