Skip to content

Commit

Permalink
fix: add tag validation to MatchTags (#410)
Browse files Browse the repository at this point in the history
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;
```
  • Loading branch information
pborn-ionos authored Feb 12, 2025
1 parent 58c106a commit bed7ddf
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 0 deletions.
2 changes: 2 additions & 0 deletions api/v1alpha1/proxmoxmachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand Down
52 changes: 52 additions & 0 deletions api/v1alpha1/proxmoxmachine_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1alpha1

import (
"context"
"strconv"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit bed7ddf

Please sign in to comment.