Skip to content

Commit

Permalink
prevent modification of original object labels
Browse files Browse the repository at this point in the history
There was a bug inside the createLabels function
that modified the labels of the original objects
instead of creating a new map. This commit fixed it.

Signed-off-by: Chanwit Kaewkasi <chanwit@gmail.com>
  • Loading branch information
chanwit committed Sep 19, 2023
1 parent d834a73 commit 41c4445
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 19 deletions.
23 changes: 11 additions & 12 deletions internal/server/polling/poll_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package polling
import (
"context"
"fmt"
bpconfig "github.com/weaveworks/tf-controller/internal/config"
"k8s.io/apimachinery/pkg/labels"
"testing"

"github.com/weaveworks/tf-controller/internal/git/provider/providerfakes"
Expand Down Expand Up @@ -197,10 +199,10 @@ func Test_poll_reconcile_objects(t *testing.T) {
expectToEqual(g, item.Spec.StoreReadablePlan, "human")
expectToEqual(g, item.Spec.ApprovePlan, "")
expectToEqual(g, item.Spec.Force, false)
expectToEqual(g, item.Spec.WriteOutputsToSecret, nil) // we don't need to use the output Secret of the plan
expectToEqual(g, item.Labels["infra.weave.works/branch-planner"], "true")
g.Expect(item.Spec.WriteOutputsToSecret).To(gomega.BeNil()) // we don't need to use the output Secret of the plan
expectToEqual(g, item.Labels[bpconfig.LabelKey], bpconfig.LabelValue)
expectToEqual(g, item.Labels["test-label"], "abc")
expectToEqual(g, item.Labels["infra.weave.works/pr-id"], fmt.Sprint(idx+1))
expectToEqual(g, item.Labels[bpconfig.LabelPRIDKey], fmt.Sprint(idx+1))
}

// Check that the Source objects are created with all expected fields.
Expand All @@ -217,9 +219,9 @@ func Test_poll_reconcile_objects(t *testing.T) {
for idx, item := range srcList.Items[1:] {
expectToEqual(g, item.Name, fmt.Sprintf("%s-%d", source.Name, idx+1))
expectToEqual(g, item.Spec.Reference.Branch, fmt.Sprintf("test-branch-%d", idx+1))
expectToEqual(g, item.Labels["infra.weave.works/branch-planner"], "true")
expectToEqual(g, item.Labels[bpconfig.LabelKey], bpconfig.LabelValue)
expectToEqual(g, item.Labels["test-label"], "123")
expectToEqual(g, item.Labels["infra.weave.works/pr-id"], fmt.Sprint(idx+1))
expectToEqual(g, item.Labels[bpconfig.LabelPRIDKey], fmt.Sprint(idx+1))
}

// Check that branch Terraform objects are updated
Expand All @@ -234,16 +236,13 @@ func Test_poll_reconcile_objects(t *testing.T) {
tfList.Items = nil

expectToSucceed(g, k8sClient.List(context.TODO(), &tfList, &client.ListOptions{
Namespace: ns.Name,
Namespace: ns.Name,
LabelSelector: labels.Set{bpconfig.LabelKey: bpconfig.LabelValue}.AsSelector(),
}))

for idx, item := range tfList.Items {
expectedSecretName := fmt.Sprintf("%s-%d", secretName, idx)
if idx == 0 {
expectedSecretName = secretName
}
for _, item := range tfList.Items {
expectToEqual(g, item.Labels["test-label"], "xyz")
expectToEqual(g, item.Spec.WriteOutputsToSecret.Name, expectedSecretName)
g.Expect(item.Spec.WriteOutputsToSecret).To(gomega.BeNil())
}

// Check that corresponding Terraform objects and Sources are deleted
Expand Down
14 changes: 7 additions & 7 deletions internal/server/polling/terraform.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,15 +174,15 @@ func (s *Server) createObjectName(name string, prID string) string {
}

func (s *Server) createLabels(labels map[string]string, originalName string, branch string, prID string) map[string]string {
if labels == nil {
labels = make(map[string]string)
resultLabels := make(map[string]string)
for k, v := range labels {
resultLabels[k] = v
}

labels[bpconfig.LabelKey] = bpconfig.LabelValue
labels[bpconfig.LabelPrimaryResourceKey] = originalName
labels[bpconfig.LabelPRIDKey] = prID

return labels
resultLabels[bpconfig.LabelKey] = bpconfig.LabelValue
resultLabels[bpconfig.LabelPrimaryResourceKey] = originalName
resultLabels[bpconfig.LabelPRIDKey] = prID
return resultLabels
}

func (s *Server) deleteTerraformAndSource(ctx context.Context, tf *infrav1.Terraform) error {
Expand Down

0 comments on commit 41c4445

Please sign in to comment.