Repository: fission/fission
Pull request #2659 (2022-12-07 07:31:58)

Fix namespace resolution with DEFAULT_NAMESPACE and other parameters set
<!--  Thanks for sending a pull request! We request you provide detailed description as much as possible. -->

## Description
<!--- Describe your changes in detail. -->
<!-- Typically try to give details of what, why and how of the PR changes. -->

## Which issue(s) this PR fixes:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
Fixes #

## Testing
<!--- Please describe in detail how you tested your changes. -->

## Checklist:
<!-- Please tick following checkboxes as per your understanding. -->
- [ ] I ran tests as well as code linting locally to verify my changes. 
- [ ] I have done manual verification of my changes, changes working as expected.
- [ ] I have added new tests to cover my changes.
- [ ] My changes follow contributing guidelines of Fission.
- [ ] I have signed all of my commits.


codecov[bot]: # [Codecov](https://codecov.io/gh/fission/fission/pull/2659?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fission) Report
> Merging [#2659](https://codecov.io/gh/fission/fission/pull/2659?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fission) (46060fa) into [main](https://codecov.io/gh/fission/fission/commit/ee623d31b2585f8507d3507ad05148ee8c39bc8d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fission) (ee623d3) will **increase** coverage by `0.16%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main    #2659      +/-   ##
==========================================
+ Coverage   19.63%   19.80%   +0.16%     
==========================================
  Files          63       63              
  Lines        7223     7227       +4     
==========================================
+ Hits         1418     1431      +13     
+ Misses       5701     5692       -9     
  Partials      104      104              
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `19.80% <100.00%> (+0.16%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fission#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/fission/fission/pull/2659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fission) | Coverage Δ | |
|---|---|---|
| [pkg/utils/namespace.go](https://codecov.io/gh/fission/fission/pull/2659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fission#diff-cGtnL3V0aWxzL25hbWVzcGFjZS5nbw==) | `61.45% <100.00%> (+10.37%)` | :arrow_up: |
| [pkg/utils/httpserver/server.go](https://codecov.io/gh/fission/fission/pull/2659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fission#diff-cGtnL3V0aWxzL2h0dHBzZXJ2ZXIvc2VydmVyLmdv) | `61.11% <0.00%> (+5.55%)` | :arrow_up: |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fission)


Patch:
diff --git a/pkg/utils/namespace.go b/pkg/utils/namespace.go
index 73a89889..1e552e8d 100644
--- a/pkg/utils/namespace.go
+++ b/pkg/utils/namespace.go
@@ -42,7 +42,7 @@ func init() {
 		FunctionNamespace: os.Getenv(ENV_FUNCTION_NAMESPACE),
 		BuiderNamespace:   os.Getenv(ENV_BUILDER_NAMESPACE),
 		DefaultNamespace:  os.Getenv(ENV_DEFAULT_NAMESPACE),
-		FissionResourceNS: getNamespaces(),
+		FissionResourceNS: GetNamespaces(),
 		Logger:            loggerfactory.GetLogger(),
 	}
 
@@ -106,27 +106,33 @@ func (nsr *NamespaceResolver) FissionNSWithOptions(option ...option) map[string]
 	return fissionResourceNS
 }
 
-func getNamespaces() map[string]string {
-	envValue := os.Getenv(ENV_ADDITIONAL_NAMESPACE)
-	if len(envValue) == 0 {
-		return map[string]string{
-			metav1.NamespaceDefault: metav1.NamespaceDefault,
-		}
+func GetNamespaces() map[string]string {
+	namespaces := make(map[string]string)
+
+	envValue := os.Getenv(ENV_DEFAULT_NAMESPACE)
+	if len(envValue) > 0 {
+		namespaces[envValue] = envValue
 	}
 
-	lstNamespaces := strings.Split(envValue, ",")
-	namespaces := make(map[string]string, len(lstNamespaces))
-	for _, namespace := range lstNamespaces {
-		//check to handle string with additional comma at the end of string. eg- ns1,ns2,
-		if namespace != "" {
-			namespaces[namespace] = namespace
+	envValue = os.Getenv(ENV_ADDITIONAL_NAMESPACE)
+	if len(envValue) > 0 {
+		lstNamespaces := strings.Split(envValue, ",")
+		for _, namespace := range lstNamespaces {
+			//check to handle string with additional comma at the end of string. eg- ns1,ns2,
+			if namespace != "" {
+				namespaces[namespace] = namespace
+			}
 		}
 	}
+
+	if len(namespaces) == 0 {
+		namespaces[metav1.NamespaceDefault] = metav1.NamespaceDefault
+	}
 	return namespaces
 }
 
 func (nsr *NamespaceResolver) GetBuilderNS(namespace string) string {
-	if nsr.FunctionNamespace == "" || nsr.BuiderNamespace == "" {
+	if nsr.BuiderNamespace == "" {
 		return namespace
 	}
 
@@ -137,7 +143,7 @@ func (nsr *NamespaceResolver) GetBuilderNS(namespace string) string {
 }
 
 func (nsr *NamespaceResolver) GetFunctionNS(namespace string) string {
-	if nsr.FunctionNamespace == "" || nsr.BuiderNamespace == "" {
+	if nsr.FunctionNamespace == "" {
 		return namespace
 	}
 


Test patch:
diff --git a/pkg/utils/namespace_test.go b/pkg/utils/namespace_test.go
index 61b55984..9620f2d0 100644
--- a/pkg/utils/namespace_test.go
+++ b/pkg/utils/namespace_test.go
@@ -1,6 +1,7 @@
 package utils
 
 import (
+	"os"
 	"testing"
 )
 
@@ -20,22 +21,52 @@ func TestNamespaceResolver(t *testing.T) {
 			},
 			{
 				name:              "should return testns2 namespace",
-				namespaceResolver: getFissionNamespaces("fission-builder", "fission-function", "testns"),
+				namespaceResolver: getFissionNamespaces("fission-builder", "fission-function", "default"),
 				namespace:         "testns2",
 				expected:          "testns2",
 			},
+			{
+				name:              "should return fission-builder namespace",
+				namespaceResolver: getFissionNamespaces("fission-builder", "", "default"),
+				namespace:         "default",
+				expected:          "fission-builder",
+			},
 			{
 				name:              "should return testns3 namespace",
-				namespaceResolver: getFissionNamespaces("", "", "testns"),
+				namespaceResolver: getFissionNamespaces("fission-builder", "", "testns"),
 				namespace:         "testns3",
 				expected:          "testns3",
 			},
+			{
+				name:              "should return testns4 namespace",
+				namespaceResolver: getFissionNamespaces("", "fission-function", "default"),
+				namespace:         "testns4",
+				expected:          "testns4",
+			},
+			{
+				name:              "should return testns5 namespace",
+				namespaceResolver: getFissionNamespaces("", "fission-function", "testns"),
+				namespace:         "testns5",
+				expected:          "testns5",
+			},
 			{
 				name:              "should return default namespace",
-				namespaceResolver: getFissionNamespaces("fission-builder", "", "default"),
+				namespaceResolver: getFissionNamespaces("", "", "default"),
 				namespace:         "default",
 				expected:          "default",
 			},
+			{
+				name:              "should return testns6 namespace",
+				namespaceResolver: getFissionNamespaces("", "", "default"),
+				namespace:         "testns6",
+				expected:          "testns6",
+			},
+			{
+				name:              "should return testns7 namespace",
+				namespaceResolver: getFissionNamespaces("", "", ""),
+				namespace:         "testns7",
+				expected:          "testns7",
+			},
 		} {
 			t.Run(test.name, func(t *testing.T) {
 				ns := test.namespaceResolver.GetBuilderNS(test.namespace)
@@ -61,22 +92,52 @@ func TestNamespaceResolver(t *testing.T) {
 			},
 			{
 				name:              "should return testns2 namespace",
-				namespaceResolver: getFissionNamespaces("fission-builder", "fission-function", "testns"),
+				namespaceResolver: getFissionNamespaces("fission-builder", "fission-function", "default"),
 				namespace:         "testns2",
 				expected:          "testns2",
 			},
+			{
+				name:              "should return fission-function namespace",
+				namespaceResolver: getFissionNamespaces("", "fission-function", "default"),
+				namespace:         "default",
+				expected:          "fission-function",
+			},
 			{
 				name:              "should return testns3 namespace",
-				namespaceResolver: getFissionNamespaces("", "", "testns"),
+				namespaceResolver: getFissionNamespaces("", "fission-function", "testns"),
 				namespace:         "testns3",
 				expected:          "testns3",
 			},
+			{
+				name:              "should return testns4 namespace",
+				namespaceResolver: getFissionNamespaces("fission-builder", "", "default"),
+				namespace:         "testns4",
+				expected:          "testns4",
+			},
+			{
+				name:              "should return testns5 namespace",
+				namespaceResolver: getFissionNamespaces("fission-builder", "", "testns"),
+				namespace:         "testns5",
+				expected:          "testns5",
+			},
 			{
 				name:              "should return default namespace",
-				namespaceResolver: getFissionNamespaces("", "fission-function", "default"),
+				namespaceResolver: getFissionNamespaces("", "", "default"),
 				namespace:         "default",
 				expected:          "default",
 			},
+			{
+				name:              "should return testns6 namespace",
+				namespaceResolver: getFissionNamespaces("", "", "default"),
+				namespace:         "testns6",
+				expected:          "testns6",
+			},
+			{
+				name:              "should return testns7 namespace",
+				namespaceResolver: getFissionNamespaces("", "", ""),
+				namespace:         "testns7",
+				expected:          "testns7",
+			},
 		} {
 			t.Run(test.name, func(t *testing.T) {
 				ns := test.namespaceResolver.GetFunctionNS(test.namespace)
@@ -115,6 +176,51 @@ func TestNamespaceResolver(t *testing.T) {
 			})
 		}
 	})
+
+	t.Run("getNamespace", func(t *testing.T) {
+		for _, test := range []struct {
+			name                string
+			defaultNamespace    string
+			additionalNamespace string
+			expected            int
+		}{
+			{
+				name:                "length of namespaces should be 1",
+				defaultNamespace:    "",
+				additionalNamespace: "",
+				expected:            1,
+			},
+			{
+				name:                "length of namespaces should be 1",
+				defaultNamespace:    "default",
+				additionalNamespace: "",
+				expected:            1,
+			},
+			{
+				name:                "length of namespaces should be 1",
+				defaultNamespace:    "default",
+				additionalNamespace: "default",
+				expected:            1,
+			},
+			{
+				name:                "length of namespaces should be 3",
+				defaultNamespace:    "default",
+				additionalNamespace: "testns1,testns2",
+				expected:            3,
+			},
+		} {
+			t.Run(test.name, func(t *testing.T) {
+				err := setNamespace(test.defaultNamespace, test.additionalNamespace)
+				if err != nil {
+					t.Fatalf("error setting environment Variable %s", err.Error())
+				}
+				ns := GetNamespaces()
+				if test.expected != len(ns) {
+					t.Errorf("expected length of namespace %d, got %d", test.expected, len(ns))
+				}
+			})
+		}
+	})
 }
 
 func getFissionNamespaces(builderNS, functionNS, defaultNS string) *NamespaceResolver {
@@ -124,3 +230,15 @@ func getFissionNamespaces(builderNS, functionNS, defaultNS string) *NamespaceRes
 		DefaultNamespace:  defaultNS,
 	}
 }
+
+func setNamespace(defaultNamespace string, additionalNamespace string) error {
+	err := os.Setenv(ENV_DEFAULT_NAMESPACE, defaultNamespace)
+	if err != nil {
+		return err
+	}
+	err = os.Setenv(ENV_ADDITIONAL_NAMESPACE, additionalNamespace)
+	if err != nil {
+		return err
+	}
+	return nil
+}