Skip to content

Commit

Permalink
feat: always store namespace in func.yaml and warn if current ns & fu…
Browse files Browse the repository at this point in the history
…nc.yaml ns is different (#1118)

* check namespace; add to func.yaml on first deploy; added tests and mock kubeconfig for testing

* change func args because only f.Namespace is used instead of the whole struct

* func comment

* misspell

* fix tests

* warning in stderr

* use context.Background

* add fake kubeconfig for new test
  • Loading branch information
gauron99 committed Jul 20, 2022
1 parent 59df756 commit 8cb7080
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 2 deletions.
3 changes: 3 additions & 0 deletions cmd/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,9 @@ created: 2009-11-10 23:00:00`,
}

func testBuilderPersistence(t *testing.T, testRegistry string, cmdBuilder func(ClientFactory) *cobra.Command) {
//add this to work with all other tests in deploy_test.go
defer WithEnvVar(t, "KUBECONFIG", fmt.Sprintf("%s/testdata/kubeconfig_deploy_namespace", cwd()))()

root, rm := Mktemp(t)
defer rm()

Expand Down
28 changes: 28 additions & 0 deletions cmd/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"knative.dev/kn-plugin-func/buildpacks"
"knative.dev/kn-plugin-func/docker"
"knative.dev/kn-plugin-func/docker/creds"
"knative.dev/kn-plugin-func/k8s"
"knative.dev/kn-plugin-func/s2i"
)

Expand Down Expand Up @@ -125,6 +126,12 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err
function.ImageDigest = imageSplit[1]
}

// add ns to func.yaml on first deploy and warn if current context differs from func.yaml
function.Namespace, err = checkNamespaceDeploy(function.Namespace, config.Namespace)
if err != nil {
return
}

function.Envs, _, err = mergeEnvs(function.Envs, config.EnvToUpdate, config.EnvToRemove)
if err != nil {
return
Expand Down Expand Up @@ -550,3 +557,24 @@ func parseImageDigest(imageSplit []string, config deployConfig, cmd *cobra.Comma

return config, nil
}

// checkNamespaceDeploy checks current namespace against func.yaml and warns if its different
// or sets namespace to be written in func.yaml if its the first deployment
func checkNamespaceDeploy(funcNamespace string, confNamespace string) (string, error) {
currNamespace, err := k8s.GetNamespace("")
if err != nil {
return funcNamespace, err
}

// If ns exists in func.yaml & NOT given via CLI (--namespace flag) & current ns does NOT match func.yaml ns
if funcNamespace != "" && confNamespace == "" && (currNamespace != funcNamespace) {
fmt.Fprintf(os.Stderr, "Warning: Current namespace '%s' does not match namespace '%s' in func.yaml. Function is deployed at '%s' namespace\n", currNamespace, funcNamespace, funcNamespace)
}

// Add current namespace to func.yaml if it is NOT set yet & NOT given via --namespace.
if funcNamespace == "" {
funcNamespace = currNamespace
}

return funcNamespace, nil
}
93 changes: 91 additions & 2 deletions cmd/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ created: 2009-11-10 23:00:00`,
errString: "remote git arguments require the --build=git flag",
},
}

defer WithEnvVar(t, "KUBECONFIG", fmt.Sprintf("%s/testdata/kubeconfig_deploy_namespace", cwd()))()
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var captureFn fn.Function
Expand Down Expand Up @@ -138,7 +140,7 @@ created: 2009-11-10 23:00:00`,
t.Fatal(err)
}

ctx := context.TODO()
ctx := context.Background()

_, err := cmd.ExecuteContextC(ctx)
if err != nil {
Expand Down Expand Up @@ -229,6 +231,8 @@ runtime: go`,
runtime: go`,
},
}

defer WithEnvVar(t, "KUBECONFIG", fmt.Sprintf("%s/testdata/kubeconfig_deploy_namespace", cwd()))()
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
deployer := mock.NewDeployer()
Expand Down Expand Up @@ -259,7 +263,7 @@ runtime: go`,
t.Fatal(err)
}

ctx := context.TODO()
ctx := context.Background()

_, err := cmd.ExecuteContextC(ctx)
if err != nil {
Expand All @@ -276,3 +280,88 @@ runtime: go`,
func TestDeploy_BuilderPersistence(t *testing.T) {
testBuilderPersistence(t, "docker.io/tigerteam", NewDeployCmd)
}

func Test_namespaceCheck(t *testing.T) {
tests := []struct {
name string
registry string
namespace string
funcFile string
expectNS string
}{
{
name: "first deployment(no ns in func.yaml), not given via cli, expect write in func.yaml",
registry: "docker.io/4141gauron3268",
expectNS: "test-ns-deploy",
funcFile: `name: test-func
runtime: go`,
},
{
name: "ns in func.yaml, not given via cli, current ns matches func.yaml",
registry: "docker.io/4141gauron3268",
expectNS: "test-ns-deploy",
funcFile: `name: test-func
namespace: "test-ns-deploy"
runtime: go`,
},
{
name: "ns in func.yaml, given via cli (always override)",
namespace: "test-ns-deploy",
expectNS: "test-ns-deploy",
registry: "docker.io/4141gauron3268",
funcFile: `name: test-func
namespace: "non-default"
runtime: go`,
},
{
name: "ns in func.yaml, not given via cli, current ns does NOT match func.yaml",
registry: "docker.io/4141gauron3268",
expectNS: "non-default",
funcFile: `name: test-func
namespace: "non-default"
runtime: go`,
},
}

// create mock kubeconfig with set namespace as 'default'
defer WithEnvVar(t, "KUBECONFIG", fmt.Sprintf("%s/testdata/kubeconfig_deploy_namespace", cwd()))()

for _, tt := range tests {

t.Run(tt.name, func(t *testing.T) {
deployer := mock.NewDeployer()
defer Fromtemp(t)()
cmd := NewDeployCmd(NewClientFactory(func() *fn.Client {
return fn.New(
fn.WithDeployer(deployer))
}))

// set namespace argument if given & reset after
cmd.SetArgs([]string{}) // Do not use test command args
viper.SetDefault("namespace", tt.namespace)
viper.SetDefault("registry", tt.registry)
defer viper.Reset()

// set test case's func.yaml
if err := os.WriteFile("func.yaml", []byte(tt.funcFile), os.ModePerm); err != nil {
t.Fatal(err)
}

ctx := context.Background()

_, err := cmd.ExecuteContextC(ctx)
if err != nil {
t.Fatalf("Got error '%s' but expected success", err)
}

fileFunction, err := fn.NewFunction(".")
if err != nil {
t.Fatalf("problem creating function: %v", err)
}

if fileFunction.Namespace != tt.expectNS {
t.Fatalf("Expected namespace '%s' but function has '%s' namespace", tt.expectNS, fileFunction.Namespace)
}
})
}
}
25 changes: 25 additions & 0 deletions cmd/testdata/kubeconfig_deploy_namespace
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
apiVersion: v1
clusters:
- cluster:
insecure-skip-tls-verify: true
server: https://example-cluster.serverless.devcluster.openshift.com:6443
name: example-cluster-serverless-devcluster-openshift-com:6443
contexts:
- context:
cluster: example-cluster-serverless-devcluster-openshift-com:6443
namespace: default
user: kube:admin/example-cluster-serverless-devcluster-openshift-com:6443
name: default/example-cluster-serverless-devcluster-openshift-com:6443/kube:admin
- context:
cluster: example-cluster-serverless-devcluster-openshift-com:6443
namespace: test-ns-deploy
user: kube:admin/example-cluster-serverless-devcluster-openshift-com:6443
name: test-ns-deploy/example-cluster-serverless-devcluster-openshift-com:6443/kube:admin
current-context: test-ns-deploy/example-cluster-serverless-devcluster-openshift-com:6443/kube:admin
kind: Config
preferences: {}
users:
- name: kubeadmin
user:
token: sha256~XXXXexample-test-hash

0 comments on commit 8cb7080

Please sign in to comment.