From 42ff26093024561c3ca7f9bd16129331c8fa1e4c Mon Sep 17 00:00:00 2001
From: Mateusz Szostok <mateusz@capact.io>
Date: Thu, 23 Dec 2021 20:18:21 +0100
Subject: [PATCH] Ensure map initialized before accessing it, create secret
 only if needed

---
 internal/k8s-engine/controller/action_service.go       | 10 ++++++++++
 internal/k8s-engine/graphql/domain/action/converter.go |  4 ++++
 .../k8s-engine/graphql/domain/action/converter_test.go |  3 +++
 .../k8s-engine/graphql/domain/action/fixtures_test.go  |  5 +++++
 4 files changed, 22 insertions(+)

diff --git a/internal/k8s-engine/controller/action_service.go b/internal/k8s-engine/controller/action_service.go
index 9aa469cf9..6d2aa36e9 100644
--- a/internal/k8s-engine/controller/action_service.go
+++ b/internal/k8s-engine/controller/action_service.go
@@ -278,6 +278,16 @@ func (a *ActionService) EnsureRunnerInputDataCreated(ctx context.Context, saName
 			return err
 		}
 
+		if !metav1.IsControlledBy(oldSecret, action) {
+			return errors.Errorf("Secret %q already exists and it is not owned by Action with the same name", key.String())
+		}
+
+		// Kubernetes allows creating Secret without data.
+		// We cannot be 100% sure that it was already initialized.
+		if oldSecret.Data == nil {
+			oldSecret.Data = map[string][]byte{}
+		}
+
 		oldSecret.Data[runnerContextSecretKey] = secret.Data[runnerContextSecretKey]
 		oldSecret.Data[runnerArgsSecretKey] = secret.Data[runnerArgsSecretKey]
 		return a.k8sCli.Update(ctx, oldSecret)
diff --git a/internal/k8s-engine/graphql/domain/action/converter.go b/internal/k8s-engine/graphql/domain/action/converter.go
index 27ed2899c..be8074e23 100644
--- a/internal/k8s-engine/graphql/domain/action/converter.go
+++ b/internal/k8s-engine/graphql/domain/action/converter.go
@@ -230,6 +230,10 @@ func (c *Converter) inputParamsFromGraphQL(in *graphql.ActionInputData, name str
 		data[ActionPolicySecretDataKey] = string(policyData)
 	}
 
+	if len(data) == 0 { // e.g. after unmarshaling we discovered that empty params were submitted
+		return nil, nil
+	}
+
 	return &v1.Secret{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       secretKind,
diff --git a/internal/k8s-engine/graphql/domain/action/converter_test.go b/internal/k8s-engine/graphql/domain/action/converter_test.go
index 8f6381f24..9a2f2a453 100644
--- a/internal/k8s-engine/graphql/domain/action/converter_test.go
+++ b/internal/k8s-engine/graphql/domain/action/converter_test.go
@@ -54,6 +54,9 @@ func TestConverter_FromGraphQLInput_HappyPath(t *testing.T) {
 			expectedModelPolicy: fixModelInputPolicy(name),
 			expectedModelSecret: fixModelInputSecret(name, false, true),
 		},
+		"Should ignore empty parameters and don't create secret": {
+			givenGQLParams: fixEmptyGQLInputParameters(),
+		},
 	}
 	for tn, tc := range tests {
 		t.Run(tn, func(t *testing.T) {
diff --git a/internal/k8s-engine/graphql/domain/action/fixtures_test.go b/internal/k8s-engine/graphql/domain/action/fixtures_test.go
index 87bfee203..ebefbf5c8 100644
--- a/internal/k8s-engine/graphql/domain/action/fixtures_test.go
+++ b/internal/k8s-engine/graphql/domain/action/fixtures_test.go
@@ -344,6 +344,11 @@ func fixGQLInputParameters() *graphql.JSON {
 	return &params
 }
 
+func fixEmptyGQLInputParameters() *graphql.JSON {
+	params := graphql.JSON(`{}`)
+	return &params
+}
+
 func fixGQLInputTypeInstances() []*graphql.InputTypeInstanceData {
 	return []*graphql.InputTypeInstanceData{
 		{