Skip to content

Commit

Permalink
Fix app-namespace usage for cluster options
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
praveenrewar committed Sep 26, 2023
1 parent 1185416 commit 2ea8ff3
Show file tree
Hide file tree
Showing 7 changed files with 206 additions and 12 deletions.
9 changes: 6 additions & 3 deletions pkg/deploy/kapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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())
Expand Down
14 changes: 11 additions & 3 deletions pkg/kubeconfig/kubeconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ type AccessInfo struct {
Name string
Namespace string

DeployNamespace string

Kubeconfig *Restricted
DangerousUsePodServiceAccount bool
}
Expand All @@ -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

Expand All @@ -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.defaultNamespace
}

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
}
8 changes: 7 additions & 1 deletion pkg/kubeconfig/kubeconfig_restricted.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
// Restricted contains a kubernetes kubeconfig as a string
type Restricted struct {
result string

defaultNamespace string
}

// NewKubeconfigRestricted takes kubeconfig yaml as input and returns kubeconfig yaml with certain fields restricted (removed).
Expand Down Expand Up @@ -65,6 +67,7 @@ func NewKubeconfigRestricted(input string) (*Restricted, error) {
})
}

defaultNamespace := "default" // TODO: Use the value from client-go directly
for _, inputCtx := range inputConfig.Contexts {
resultConfig.Contexts = append(resultConfig.Contexts, clientcmd.NamedContext{
Name: inputCtx.Name,
Expand All @@ -74,14 +77,17 @@ func NewKubeconfigRestricted(input string) (*Restricted, error) {
Namespace: inputCtx.Context.Namespace,
},
})
if inputCtx.Name == inputConfig.CurrentContext && inputCtx.Context.Namespace != "" {
defaultNamespace = inputCtx.Context.Namespace
}
}

bs, err := yaml.Marshal(resultConfig)
if err != nil {
return nil, fmt.Errorf("Marshaling kubeconfig: %s", err)
}

return &Restricted{string(bs)}, nil
return &Restricted{string(bs), defaultNamespace}, nil
}

// AsYAML returns a string formatted kubernetes kubeconfig
Expand Down
6 changes: 4 additions & 2 deletions pkg/kubeconfig/kubeconfig_secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions pkg/kubeconfig/service_accounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
133 changes: 133 additions & 0 deletions test/e2e/kappcontroller/default_namespace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package kappcontroller

import (
"fmt"
"os"
"strings"
"testing"

Expand Down Expand Up @@ -71,6 +72,138 @@ spec:
kubectl.RunWithOpts([]string{"get", "configmap", name + ".app", "-n", env.Namespace}, e2e.RunOpts{NoNamespace: true})
}

func Test_AppDefaultNamespace_WithTargetCluster(t *testing.T) {
targetClusterKubeconfig := os.Getenv("TEST_E2E_TARGET_CLUSTER_KUBECONFIG")
if targetClusterKubeconfig == "" {
t.Skip("Skipping test as target cluster kubeconfig is not set")
}

kubeconfigFile, err := os.CreateTemp("", "e2e-kubeconfig-*")
assert.NoError(t, err)
defer os.Remove(kubeconfigFile.Name())

_, err = kubeconfigFile.Write([]byte(targetClusterKubeconfig))
assert.NoError(t, err)

env := e2e.BuildEnv(t)
logger := e2e.Logger{}
kapp := e2e.Kapp{t, env.Namespace, logger}
kubectl := e2e.Kubectl{t, env.Namespace, logger}

name := "app-default-namespace-target-cluster"
defaultNamespace := "e2e-default-namespace"
clusterNamespace := "e2e-cluster-namespace"
namespaceApp := "namespace-app"
secretName := "e2e-kubeconfig-secret"

cleanUp := func() {
kapp.Run([]string{"delete", "-a", name})

}
cleanUpTargetCluster := func() {
kapp.RunWithOpts([]string{"delete", "-a", namespaceApp, "--kubeconfig", kubeconfigFile.Name()}, e2e.RunOpts{NoNamespace: true})
}

cleanUp()
cleanUpTargetCluster()
defer cleanUp()
defer cleanUpTargetCluster()

namespaceYAML := fmt.Sprintf(`---
apiVersion: v1
kind: Namespace
metadata:
name: %s
---
apiVersion: v1
kind: Namespace
metadata:
name: %s`, defaultNamespace, clusterNamespace)

secret := e2e.Secrets{secretName, env.Namespace, targetClusterKubeconfig}
appYAML := `---
apiVersion: kappctrl.k14s.io/v1alpha1
kind: App
metadata:
name: %s
namespace: %s
annotations:
kapp.k14s.io/change-group: "kappctrl-e2e.k14s.io/apps"
spec:
cluster:
namespace: %s
kubeconfigSecretRef:
name: %s
defaultNamespace: %s
fetch:
- inline:
paths:
file.yml: |
apiVersion: v1
kind: ConfigMap
metadata:
name: my-cm
data:
key: value
template:
- ytt: {}
deploy:
- kapp: {}`

// create test namespaces on target cluster
kapp.RunWithOpts([]string{"deploy", "-a", namespaceApp, "-f", "-", "--kubeconfig", kubeconfigFile.Name()}, e2e.RunOpts{NoNamespace: true, StdinReader: strings.NewReader(namespaceYAML)})

// Provide both _defaultNamespace_ and _cluster.namespace_
_, err = kapp.RunWithOpts([]string{"deploy", "-a", name, "-f", "-"}, e2e.RunOpts{AllowError: true,
StdinReader: strings.NewReader(fmt.Sprintf(appYAML, name, env.Namespace, clusterNamespace, secretName, defaultNamespace) + secret.ForTargetCluster())})
assert.NoError(t, err, "Expected app deploy to succeed, it did not")

// Assert that app resources are in defaultNamespace
kubectl.RunWithOpts([]string{"get", "configmap", "my-cm", "-n", defaultNamespace, "--kubeconfig", kubeconfigFile.Name()}, e2e.RunOpts{NoNamespace: true})

// Assert that kapp metaconfigmap is in cluster.namespace
kubectl.RunWithOpts([]string{"get", "configmap", name + ".app", "-n", clusterNamespace, "--kubeconfig", kubeconfigFile.Name()}, e2e.RunOpts{NoNamespace: true})

cleanUp()

// Provide only _cluster.namespace_
_, err = kapp.RunWithOpts([]string{"deploy", "-a", name, "-f", "-"}, e2e.RunOpts{AllowError: true,
StdinReader: strings.NewReader(fmt.Sprintf(appYAML, name, env.Namespace, clusterNamespace, secretName, "") + secret.ForTargetCluster())})
assert.NoError(t, err, "Expected app deploy to succeed, it did not")

// Assert that app resources are in cluster.namespace
kubectl.RunWithOpts([]string{"get", "configmap", "my-cm", "-n", clusterNamespace, "--kubeconfig", kubeconfigFile.Name()}, e2e.RunOpts{NoNamespace: true})

// Assert that kapp metaconfigmap is in cluster.namespace
kubectl.RunWithOpts([]string{"get", "configmap", name + ".app", "-n", clusterNamespace, "--kubeconfig", kubeconfigFile.Name()}, e2e.RunOpts{NoNamespace: true})

cleanUp()

// Provide only _defaultNamespace_
_, err = kapp.RunWithOpts([]string{"deploy", "-a", name, "-f", "-"}, e2e.RunOpts{AllowError: true,
StdinReader: strings.NewReader(fmt.Sprintf(appYAML, name, env.Namespace, "", secretName, defaultNamespace) + secret.ForTargetCluster())})
assert.NoError(t, err, "Expected app deploy to succeed, it did not")

// Assert that app resources are in defaultNamespace
kubectl.RunWithOpts([]string{"get", "configmap", "my-cm", "-n", defaultNamespace, "--kubeconfig", kubeconfigFile.Name()}, e2e.RunOpts{NoNamespace: true})

// Assert that kapp metaconfigmap is in kubeconfig preferred namespace (default)
kubectl.RunWithOpts([]string{"get", "configmap", name + ".app", "-n", "default", "--kubeconfig", kubeconfigFile.Name()}, e2e.RunOpts{NoNamespace: true})

cleanUp()

// Do not provide _defaultNamespace_ and _cluster.namespace_
_, err = kapp.RunWithOpts([]string{"deploy", "-a", name, "-f", "-"}, e2e.RunOpts{AllowError: true,
StdinReader: strings.NewReader(fmt.Sprintf(appYAML, name, env.Namespace, "", secretName, "") + secret.ForTargetCluster())})
assert.NoError(t, err, "Expected app deploy to succeed, it did not")

// Assert that app resources are in kubeconfig preferred namespace (default)
kubectl.RunWithOpts([]string{"get", "configmap", "my-cm", "-n", "default", "--kubeconfig", kubeconfigFile.Name()}, e2e.RunOpts{NoNamespace: true})

// Assert that kapp metaconfigmap is in kubeconfig preferred namespace (default)
kubectl.RunWithOpts([]string{"get", "configmap", name + ".app", "-n", "default", "--kubeconfig", kubeconfigFile.Name()}, e2e.RunOpts{NoNamespace: true})
}

func Test_PackageInstall_DefaultNamespace(t *testing.T) {
env := e2e.BuildEnv(t)
logger := e2e.Logger{}
Expand Down
41 changes: 41 additions & 0 deletions test/e2e/secrets.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2023 VMware, Inc.
// SPDX-License-Identifier: Apache-2.0

package e2e

import (
"fmt"
"strings"
)

// Secrets represents a secret with target cluster kubeconfig
type Secrets struct {
Name string
Namespace string
Kubeconfig string
}

// ForTargetCluster can be used to get secret with target cluster kubeconfig
func (s Secrets) ForTargetCluster() string {
indentedKubeconfig := ""
for _, line := range strings.Split(s.Kubeconfig, "\n") {
if line != "" {
indentedKubeconfig += " " + line + "\n"
}
}
return fmt.Sprintf(`
---
apiVersion: v1
kind: Secret
metadata:
name: %s
namespace: %s
annotations:
kapp.k14s.io/change-rule.apps: "delete after deleting kappctrl-e2e.k14s.io/apps"
kapp.k14s.io/change-rule.instpkgs: "delete after deleting kappctrl-e2e.k14s.io/packageinstalls"
type: Opaque
stringData:
value: |
%s
`, s.Name, s.Namespace, indentedKubeconfig)
}

0 comments on commit 2ea8ff3

Please sign in to comment.