diff --git a/assets/testdata/.env.initium.empty b/assets/testdata/.env.initium.empty new file mode 100644 index 00000000..e69de29b diff --git a/assets/testdata/.env.secretref.invalid1 b/assets/testdata/.env.secretref.initium.invalid1 similarity index 100% rename from assets/testdata/.env.secretref.invalid1 rename to assets/testdata/.env.secretref.initium.invalid1 diff --git a/assets/testdata/.env.secretref.invalid3 b/assets/testdata/.env.secretref.initium.invalid2 similarity index 100% rename from assets/testdata/.env.secretref.invalid3 rename to assets/testdata/.env.secretref.initium.invalid2 diff --git a/assets/testdata/.env.secretref.invalid2 b/assets/testdata/.env.secretref.invalid2 deleted file mode 100644 index 3e56365c..00000000 --- a/assets/testdata/.env.secretref.invalid2 +++ /dev/null @@ -1,2 +0,0 @@ -# Wrong format -MOCK5=kubernetessecretname/kubernetesecretkey diff --git a/src/services/k8s/knative.go b/src/services/k8s/knative.go index b736880a..03c3f222 100644 --- a/src/services/k8s/knative.go +++ b/src/services/k8s/knative.go @@ -3,7 +3,9 @@ package k8s import ( "bytes" "context" + "errors" "fmt" + "os" "path" "strings" "text/template" @@ -143,22 +145,10 @@ func setSecretEnv(manifest *servingv1.Service, SecretRefEnvFile string, manifest } func validateSecretEnvVar(secretEnvVar corev1.EnvVar) error { - invalidChars := []string{ - "\"", - "'", - } - // Mandatory char if !strings.Contains(secretEnvVar.Value, "/") { return fmt.Errorf("Invalid secret format for '%s'. Missing '/' char. Value must be in the format /, instead of '%s'", secretEnvVar.Name, secretEnvVar.Value) } - - // Invalid chars - for _, value := range invalidChars { - if strings.Contains(secretEnvVar.Value, value) { - return fmt.Errorf("Invalid secret format for '%s'. Invalid char found (\", ')", secretEnvVar.Name) - } - } return nil } @@ -173,13 +163,18 @@ func setEnv(manifest *servingv1.Service, envFile string, manifestEnvVars map[str func loadEnvFile(envFile string, manifestEnvVars map[string]string) ([]corev1.EnvVar, error) { var envVarList []corev1.EnvVar + + if _, err := os.Stat(envFile); errors.Is(err, os.ErrNotExist) { + return nil, nil + } + envVariables, err := godotenv.Read(envFile) if err != nil { return nil, fmt.Errorf("Error loading .env file. '%s' already set", err) } if len(envVariables) > 0 { - for key, value := range envVariables { + for key, value := range envVariables { if manifestEnvVars[key] == "" { manifestEnvVars[key] = value envVar := corev1.EnvVar{ diff --git a/src/services/k8s/knative_test.go b/src/services/k8s/knative_test.go index 014b9827..bb4c94f4 100644 --- a/src/services/k8s/knative_test.go +++ b/src/services/k8s/knative_test.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path" + "strings" "testing" "gotest.tools/v3/assert" @@ -118,16 +119,6 @@ func TestLoadManifestForPrivateService(t *testing.T) { assert.Assert(t, labels[visibilityLabel] == visibilityLabelPrivateValue) } -// TODO: New use cases: -// 1 -> All happy and working -// 2 -> Invalid chars -// 3 -> Mandatory '/' char -// 4 -> Conflicting env vars between files -// 5 -> Invalid dotenv format -// 6 -> Empty files -// 7 -> only .env is empty -// 8 -> only .env.secretref is empty - func TestLoadManifestEnvironmentVariables(t *testing.T) { envVariablesFromFile, err := godotenv.Read(envTestFile) if err != nil { @@ -150,3 +141,37 @@ func TestLoadManifestEnvironmentVariables(t *testing.T) { assert.Assert(t, len(envVariablesFromFile) == 0, "Missing environment variables: %s", envVariablesFromFile ) assert.Assert(t, len(secretRefEnvVariablesFromFile) == 0, "Missing secret environment variables: %s", secretRefEnvVariablesFromFile) } + +func TestLoadManifestEnvironmentVariablesInvalidFormat(t *testing.T) { + invalidEnvTestFile := path.Join(root, "assets/testdata/.env.initium.invalid") + serviceManifest, err := LoadManifest(namespace, commitSha, proj, dockerImage, invalidEnvTestFile, secretRefEnvTestFile) + assert.Assert(t, err != nil && strings.Contains(err.Error(), "Error loading .env file"), "There should be a validation error when missing a mandatory character" ) + assert.Assert(t, serviceManifest == nil, "Expected nil manifest, got %v", serviceManifest) +} + +func TestLoadManifestSecretRefEnvironmentMandatoryChars(t *testing.T) { + invalidSecretRefEnvTestFile := path.Join(root, "assets/testdata/.env.secretref.initium.invalid2") + serviceManifest, err := LoadManifest(namespace, commitSha, proj, dockerImage, envTestFile, invalidSecretRefEnvTestFile) + assert.Assert(t, err != nil && strings.Contains(err.Error(), "Value must be in the format /"), "There should be a validation error when missing a mandatory character" ) + assert.Assert(t, serviceManifest == nil, "Expected nil manifest, got %v", serviceManifest) +} + +func TestLoadManifestSecretRefEnvironmentConflict(t *testing.T) { + invalidSecretRefEnvTestFile := path.Join(root, "assets/testdata/.env.secretref.conflictingvar") + serviceManifest, err := LoadManifest(namespace, commitSha, proj, dockerImage, envTestFile, invalidSecretRefEnvTestFile) + assert.Assert(t, err != nil && strings.Contains(err.Error(), "Conflicting environment variable"), "There should be a validation error when missing a mandatory character" ) + assert.Assert(t, serviceManifest == nil, "Expected nil manifest, got %v", serviceManifest) +} + +func TestLoadManifestSecretEnvironmentVariablesFileDoesNotExist(t *testing.T) { + nonExistingEnvTestFile := "idontexist" + nonExistingSecretRefEnvTestFile := "idontexist" + serviceManifest, err := LoadManifest(namespace, commitSha, proj, dockerImage, nonExistingEnvTestFile, nonExistingSecretRefEnvTestFile) + assert.Assert(t, serviceManifest != nil, "Expected maifest to be created without issues. Dotenv files are optional. Err: %s", err) +} + +func TestLoadManifestSecretEnvironmentVariablesEmptyFile(t *testing.T) { + emtpyFile := path.Join(root, "assets/testdata/.env.initium.empty") + serviceManifest, err := LoadManifest(namespace, commitSha, proj, dockerImage, emtpyFile, emtpyFile) + assert.Assert(t, serviceManifest != nil, "Expected maifest to be created without issues. Dotenv files are optional. Err: %s", err) +}