From 2b476cfb74d1c1239f0925a19f61ac29ba464a5e Mon Sep 17 00:00:00 2001 From: knrt10 Date: Mon, 28 Dec 2020 12:50:02 +0530 Subject: [PATCH 1/3] Packet: fix labels and taints with spaced value Now user cannot have spaced key or value in labels, taints map for worker pools. Add tests. --- pkg/platform/packet/packet.go | 32 ++++++++++++++++++++++++++++++ pkg/platform/packet/packet_test.go | 28 ++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/pkg/platform/packet/packet.go b/pkg/platform/packet/packet.go index 21c07909c..fd97a1a94 100644 --- a/pkg/platform/packet/packet.go +++ b/pkg/platform/packet/packet.go @@ -428,6 +428,7 @@ func (c *config) checkValidConfig() hcl.Diagnostics { diagnostics = append(diagnostics, c.checkWorkerPoolNamesUnique()...) diagnostics = append(diagnostics, c.checkReservationIDs()...) diagnostics = append(diagnostics, c.validateOSVersion()...) + diagnostics = append(diagnostics, c.checkWorkerPoolLabelsAndTaints()...) if c.ConntrackMaxPerCore < 0 { diagnostics = append(diagnostics, &hcl.Diagnostic{ @@ -525,6 +526,37 @@ func (c *config) validateOSVersion() hcl.Diagnostics { return diagnostics } +// checkWorkerPoolLabelsAndTaints verifies that all worker pool labels +// and taints are in correct format. Neither key nor value of labels +// and taints map should have empty space in them. +func (c *config) checkWorkerPoolLabelsAndTaints() hcl.Diagnostics { + var diagnostics hcl.Diagnostics + + for _, w := range c.WorkerPools { + for k, v := range w.Labels { + if strings.Contains(k, " ") || strings.Contains(v, " ") { + diagnostics = append(diagnostics, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Worker pools labels map should not contain empty spaces", + Detail: fmt.Sprintf("Worker pool %q label with key %q is incorrect", w.Name, k), + }) + } + } + + for k, v := range w.Taints { + if strings.Contains(k, " ") || strings.Contains(v, " ") { + diagnostics = append(diagnostics, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Worker pools taints map should not contain empty spaces", + Detail: fmt.Sprintf("Worker pool %q taints with key %q is incorrect", w.Name, k), + }) + } + } + } + + return diagnostics +} + // checkEachReservation checks that hardware reservations are in the correct // format and, when it will cause problems, that reservation IDs values in this // pool are not mixed between using "next-available" and specific UUIDs, as this diff --git a/pkg/platform/packet/packet_test.go b/pkg/platform/packet/packet_test.go index 057710eff..9edb83a83 100644 --- a/pkg/platform/packet/packet_test.go +++ b/pkg/platform/packet/packet_test.go @@ -355,3 +355,31 @@ func TestTerraformAddDeps(t *testing.T) { }) } } + +func TestCheckWorkerPoolLabelsWithSpacedValue(t *testing.T) { + c := config{ + WorkerPools: []workerPool{ + { + Labels: map[string]string{"foo-1": "bar "}, + }, + }, + } + + if d := c.checkWorkerPoolLabelsAndTaints(); !d.HasErrors() { + t.Error("Should fail with space in worker pool labels") + } +} + +func TestCheckWorkerPoolTaintsWithSpacedValue(t *testing.T) { + c := config{ + WorkerPools: []workerPool{ + { + Taints: map[string]string{"foo-1": "bar "}, + }, + }, + } + + if d := c.checkWorkerPoolLabelsAndTaints(); !d.HasErrors() { + t.Error("Should fail with space in worker pool taints") + } +} From a07ebecc4f929335172c6df103204f624efba5a8 Mon Sep 17 00:00:00 2001 From: knrt10 Date: Mon, 28 Dec 2020 12:58:22 +0530 Subject: [PATCH 2/3] aws: fix labels and taints with spaced value Now user cannot have spaced key or value in labels, taints map for worker pools. Add tests. --- pkg/platform/aws/aws.go | 33 +++++++++++++++++++++++++++ pkg/platform/aws/aws_internal_test.go | 28 +++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/pkg/platform/aws/aws.go b/pkg/platform/aws/aws.go index b16232973..aeaa53196 100644 --- a/pkg/platform/aws/aws.go +++ b/pkg/platform/aws/aws.go @@ -19,6 +19,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "text/template" "github.com/hashicorp/hcl/v2" @@ -261,6 +262,7 @@ func (c *config) checkValidConfig() hcl.Diagnostics { diagnostics = append(diagnostics, c.checkWorkerPoolNamesUnique()...) diagnostics = append(diagnostics, c.checkNameSizes()...) diagnostics = append(diagnostics, c.checkLBPortsUnique()...) + diagnostics = append(diagnostics, c.checkWorkerPoolLabelsAndTaints()...) if c.ConntrackMaxPerCore < 0 { diagnostics = append(diagnostics, &hcl.Diagnostic{ @@ -324,6 +326,37 @@ func (c *config) checkLBPortsUnique() hcl.Diagnostics { return diagnostics } +// checkWorkerPoolLabelsAndTaints verifies that all worker pool labels +// and taints are in correct format. Neither key nor value of labels +// and taints map should have empty space in them. +func (c *config) checkWorkerPoolLabelsAndTaints() hcl.Diagnostics { + var diagnostics hcl.Diagnostics + + for _, w := range c.WorkerPools { + for k, v := range w.Labels { + if strings.Contains(k, " ") || strings.Contains(v, " ") { + diagnostics = append(diagnostics, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Worker pools labels map should not contain empty spaces", + Detail: fmt.Sprintf("Worker pool %q label with key %q is incorrect", w.Name, k), + }) + } + } + + for k, v := range w.Taints { + if strings.Contains(k, " ") || strings.Contains(v, " ") { + diagnostics = append(diagnostics, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Worker pools taints map should not contain empty spaces", + Detail: fmt.Sprintf("Worker pool %q taints with key %q is incorrect", w.Name, k), + }) + } + } + } + + return diagnostics +} + // checkNameSizes checks the size of names since AWS has a limit of 32 // characters on resources. func (c *config) checkNameSizes() hcl.Diagnostics { diff --git a/pkg/platform/aws/aws_internal_test.go b/pkg/platform/aws/aws_internal_test.go index 7ab0200e0..8e8d71b9f 100644 --- a/pkg/platform/aws/aws_internal_test.go +++ b/pkg/platform/aws/aws_internal_test.go @@ -222,3 +222,31 @@ func TestConfigurationIsValidWhen(t *testing.T) { }) } } + +func TestCheckWorkerPoolLabelsWithSpacedValue(t *testing.T) { + c := config{ + WorkerPools: []workerPool{ + { + Labels: map[string]string{"foo-1": "bar "}, + }, + }, + } + + if d := c.checkWorkerPoolLabelsAndTaints(); !d.HasErrors() { + t.Error("Should fail with space in worker pool labels") + } +} + +func TestCheckWorkerPoolTaintsWithSpacedValue(t *testing.T) { + c := config{ + WorkerPools: []workerPool{ + { + Taints: map[string]string{"foo-1": "bar "}, + }, + }, + } + + if d := c.checkWorkerPoolLabelsAndTaints(); !d.HasErrors() { + t.Error("Should fail with space in worker pool taints") + } +} From e9fc218814dbd46c511a7fa03aaeacc91da15400 Mon Sep 17 00:00:00 2001 From: knrt10 Date: Mon, 28 Dec 2020 13:20:01 +0530 Subject: [PATCH 3/3] tinkerbell: fix labels && taints with spaced value Now user cannot have spaced key or value in labels, taints map for worker pools. Add tests. closes: #1280 --- pkg/platform/tinkerbell/tinkerbell.go | 32 ++++++++++++++++++++++ pkg/platform/tinkerbell/tinkerbell_test.go | 28 +++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/pkg/platform/tinkerbell/tinkerbell.go b/pkg/platform/tinkerbell/tinkerbell.go index 621d82932..d246a80f7 100644 --- a/pkg/platform/tinkerbell/tinkerbell.go +++ b/pkg/platform/tinkerbell/tinkerbell.go @@ -230,6 +230,7 @@ func (c *Config) Validate() hcl.Diagnostics { d = append(d, platform.WorkerPoolNamesUnique(x)...) d = append(d, c.validateRequiredFields()...) + d = append(d, c.CheckWorkerPoolLabelsAndTaints()...) return d } @@ -288,3 +289,34 @@ func (c *Config) validateRequiredFields() hcl.Diagnostics { return d } + +// CheckWorkerPoolLabelsAndTaints verifies that all worker pool labels +// and taints are in correct format. Neither key nor value of labels +// and taints map should have empty space in them. +func (c *Config) CheckWorkerPoolLabelsAndTaints() hcl.Diagnostics { + var diagnostics hcl.Diagnostics + + for _, w := range c.WorkerPools { + for k, v := range w.Labels { + if strings.Contains(k, " ") || strings.Contains(v, " ") { + diagnostics = append(diagnostics, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Worker pools labels map should not contain empty spaces", + Detail: fmt.Sprintf("Worker pool %q label with key %q is incorrect", w.PoolName, k), + }) + } + } + + for k, v := range w.Taints { + if strings.Contains(k, " ") || strings.Contains(v, " ") { + diagnostics = append(diagnostics, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Worker pools taints map should not contain empty spaces", + Detail: fmt.Sprintf("Worker pool %q taints with key %q is incorrect", w.PoolName, k), + }) + } + } + } + + return diagnostics +} diff --git a/pkg/platform/tinkerbell/tinkerbell_test.go b/pkg/platform/tinkerbell/tinkerbell_test.go index c23f920b9..06603c38e 100644 --- a/pkg/platform/tinkerbell/tinkerbell_test.go +++ b/pkg/platform/tinkerbell/tinkerbell_test.go @@ -244,3 +244,31 @@ func TestMeta(t *testing.T) { t.Errorf("Expected %d nodes, got %d", expectedNodes, m.ExpectedNodes) } } + +func TestCheckWorkerPoolLabelsWithSpacedValue(t *testing.T) { + c := &tinkerbell.Config{ + WorkerPools: []tinkerbell.WorkerPool{ + { + Labels: map[string]string{"foo-1": "bar "}, + }, + }, + } + + if d := c.CheckWorkerPoolLabelsAndTaints(); !d.HasErrors() { + t.Error("Should fail with space in worker pool labels") + } +} + +func TestCheckWorkerPoolTaintsWithSpacedValue(t *testing.T) { + c := &tinkerbell.Config{ + WorkerPools: []tinkerbell.WorkerPool{ + { + Taints: map[string]string{"foo-1": "bar "}, + }, + }, + } + + if d := c.CheckWorkerPoolLabelsAndTaints(); !d.HasErrors() { + t.Error("Should fail with space in worker pool taints") + } +}