From bed7ddfbd4b49949d68bb334a4e34ea89f89f329 Mon Sep 17 00:00:00 2001 From: Philipp Born <98814235+pborn-ionos@users.noreply.github.com> Date: Wed, 12 Feb 2025 15:02:36 +0100 Subject: [PATCH] fix: add tag validation to MatchTags (#410) this fixes a small oversight in #343 and actually validates the individual tags against what is accepted by the API https://github.com/proxmox/pve-common/blob/master/src/PVE/JSONSchema.pm#L706 ``` our $PVE_TAG_RE = qr/[a-z0-9_][a-z0-9_\-\+\.]*/i; ``` --- api/v1alpha1/proxmoxmachine_types.go | 2 + api/v1alpha1/proxmoxmachine_types_test.go | 52 +++++++++++++++++++ ...ture.cluster.x-k8s.io_proxmoxclusters.yaml | 2 + ...ster.x-k8s.io_proxmoxclustertemplates.yaml | 2 + ...ture.cluster.x-k8s.io_proxmoxmachines.yaml | 2 + ...ster.x-k8s.io_proxmoxmachinetemplates.yaml | 2 + 6 files changed, 62 insertions(+) diff --git a/api/v1alpha1/proxmoxmachine_types.go b/api/v1alpha1/proxmoxmachine_types.go index c58b7afb..025cf498 100644 --- a/api/v1alpha1/proxmoxmachine_types.go +++ b/api/v1alpha1/proxmoxmachine_types.go @@ -229,6 +229,8 @@ type TemplateSelector struct { // Passed tags must be an exact 1:1 match with the tags on the template you want to use. // If multiple VM templates with the same set of tags are found, provisioning will fail. // + // +listType=set + // +kubebuilder:validation:items:Pattern=`^(?i)[a-z0-9_][a-z0-9_\-\+\.]*$` // +kubebuilder:validation:MinItems=1 MatchTags []string `json:"matchTags"` } diff --git a/api/v1alpha1/proxmoxmachine_types_test.go b/api/v1alpha1/proxmoxmachine_types_test.go index bbf4c158..5684747c 100644 --- a/api/v1alpha1/proxmoxmachine_types_test.go +++ b/api/v1alpha1/proxmoxmachine_types_test.go @@ -18,6 +18,7 @@ package v1alpha1 import ( "context" + "strconv" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -86,6 +87,57 @@ var _ = Describe("ProxmoxMachine Test", func() { Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("should have at least 1 items"))) }) + + It("Should only allow valid MatchTags", func() { + testCases := []struct { + tag string + expectErrror bool + errorMessage string + }{ + // Valid Tags + {"valid_tag", false, ""}, + {"Valid-Tag", false, ""}, + {"valid.tag", false, ""}, + {"VALID+TAG", false, ""}, + {"123tag", false, ""}, + {"tag123", false, ""}, + {"tag_with-hyphen", false, ""}, + {"tag.with.plus+_and-hyphen", false, ""}, + {"_tag_with_underscore", false, ""}, + + // Invalid Tags + {"", true, "in body should match"}, // Empty string + {"-invalid", true, "in body should match"}, // Starts with a hyphen + {"+invalid", true, "in body should match"}, // Starts with a plus + {".invalid", true, "in body should match"}, // Starts with a dot + {" invalid", true, "in body should match"}, // Starts with a space + {"invalid!", true, "in body should match"}, // Contains an exclamation mark + {"invalid@", true, "in body should match"}, // Contains an at symbol + {"invalid#", true, "in body should match"}, // Contains a hash symbol + {"inval id", true, "in body should match"}, // Contains a whitespace + } + + // Iterate through each test case + for i, testCase := range testCases { + // Create a new ProxmoxMachine object for each test case + dm := defaultMachine() + + // Set the name of the machine to a unique value based on the test case index + dm.ObjectMeta.Name = "test-machine-" + strconv.Itoa(i) + + // Set the template selector to match the tag from the test case + dm.Spec.TemplateSource.SourceNode = "" + dm.Spec.TemplateSource.TemplateID = nil + dm.Spec.TemplateSelector = &TemplateSelector{MatchTags: []string{testCase.tag}} + + // Run test + if !testCase.expectErrror { + Expect(k8sClient.Create(context.Background(), dm)).To(Succeed()) + } else { + Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring(testCase.errorMessage))) + } + } + }) }) Context("Disks", func() { diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclusters.yaml index 50777736..602189f6 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclusters.yaml @@ -569,9 +569,11 @@ spec: Passed tags must be an exact 1:1 match with the tags on the template you want to use. If multiple VM templates with the same set of tags are found, provisioning will fail. items: + pattern: ^(?i)[a-z0-9_][a-z0-9_\-\+\.]*$ type: string minItems: 1 type: array + x-kubernetes-list-type: set required: - matchTags type: object diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclustertemplates.yaml index f5b2a1e1..11b27d96 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclustertemplates.yaml @@ -610,9 +610,11 @@ spec: Passed tags must be an exact 1:1 match with the tags on the template you want to use. If multiple VM templates with the same set of tags are found, provisioning will fail. items: + pattern: ^(?i)[a-z0-9_][a-z0-9_\-\+\.]*$ type: string minItems: 1 type: array + x-kubernetes-list-type: set required: - matchTags type: object diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml index c3376297..06bc1b90 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml @@ -537,9 +537,11 @@ spec: Passed tags must be an exact 1:1 match with the tags on the template you want to use. If multiple VM templates with the same set of tags are found, provisioning will fail. items: + pattern: ^(?i)[a-z0-9_][a-z0-9_\-\+\.]*$ type: string minItems: 1 type: array + x-kubernetes-list-type: set required: - matchTags type: object diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml index c07d2750..70c0ff47 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml @@ -569,9 +569,11 @@ spec: Passed tags must be an exact 1:1 match with the tags on the template you want to use. If multiple VM templates with the same set of tags are found, provisioning will fail. items: + pattern: ^(?i)[a-z0-9_][a-z0-9_\-\+\.]*$ type: string minItems: 1 type: array + x-kubernetes-list-type: set required: - matchTags type: object