Skip to content

Commit

Permalink
Added error in case try to supply custom label with name of system la…
Browse files Browse the repository at this point in the history
…bel during install/upgrade

Signed-off-by: Dmitry Chepurovskiy <me@dm3ch.net>
  • Loading branch information
dm3ch committed Jul 25, 2023
1 parent 6853c3e commit 7b13ac9
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 1 deletion.
5 changes: 5 additions & 0 deletions pkg/action/install.go
Expand Up @@ -257,6 +257,11 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma
return nil, err
}

fmt.Println(driver.ContainsSystemLabels(i.Labels))
if driver.ContainsSystemLabels(i.Labels) {
return nil, fmt.Errorf("user suplied labels contains system reserved label name. System labels: %+v", driver.GetSystemLabels())
}

rel := i.createRelease(chrt, vals, i.Labels)

var manifestDoc *bytes.Buffer
Expand Down
15 changes: 15 additions & 0 deletions pkg/action/install_test.go
Expand Up @@ -732,3 +732,18 @@ func TestInstallWithLabels(t *testing.T) {

is.Equal(instAction.Labels, res.Labels)
}

func TestInstallWithSystemLabels(t *testing.T) {
is := assert.New(t)
instAction := installAction(t)
instAction.Labels = map[string]string{
"owner": "val1",
"key2": "val2",
}
_, err := instAction.Run(buildChart(), nil)
if err == nil {
t.Fatal("expected an error")
}

is.Equal(fmt.Errorf("user suplied labels contains system reserved label name. System labels: %+v", driver.GetSystemLabels()), err)
}
5 changes: 5 additions & 0 deletions pkg/action/upgrade.go
Expand Up @@ -239,6 +239,11 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin
return nil, nil, err
}

fmt.Println(driver.ContainsSystemLabels(u.Labels))
if driver.ContainsSystemLabels(u.Labels) {
return nil, nil, fmt.Errorf("user suplied labels contains system reserved label name. System labels: %+v", driver.GetSystemLabels())
}

// Store an upgraded release.
upgradedRelease := &release.Release{
Name: name,
Expand Down
31 changes: 31 additions & 0 deletions pkg/action/upgrade_test.go
Expand Up @@ -24,6 +24,7 @@ import (
"time"

"helm.sh/helm/v3/pkg/chart"
"helm.sh/helm/v3/pkg/storage/driver"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -451,3 +452,33 @@ func TestUpgradeRelease_Labels(t *testing.T) {
is.Equal(initialRes.Info.Status, release.StatusSuperseded)
is.Equal(initialRes.Labels, rel.Labels)
}

func TestUpgradeRelease_SystemLabels(t *testing.T) {
is := assert.New(t)
upAction := upgradeAction(t)

rel := releaseStub()
rel.Name = "labels"
// It's needed to check that suppressed release would keep original labels
rel.Labels = map[string]string{
"key1": "val1",
"key2": "val2.1",
}
rel.Info.Status = release.StatusDeployed

err := upAction.cfg.Releases.Create(rel)
is.NoError(err)

upAction.Labels = map[string]string{
"key1": "null",
"key2": "val2.2",
"owner": "val3",
}
// setting newValues and upgrading
_, err = upAction.Run(rel.Name, buildChart(), nil)
if err == nil {
t.Fatal("expected an error")
}

is.Equal(fmt.Errorf("user suplied labels contains system reserved label name. System labels: %+v", driver.GetSystemLabels()), err)
}
16 changes: 15 additions & 1 deletion pkg/storage/driver/util.go
Expand Up @@ -88,7 +88,7 @@ func decodeRelease(data string) (*rspb.Release, error) {

// Checks if label is system
func isSystemLabel(key string) bool {
for _, v := range systemLabels {
for _, v := range GetSystemLabels() {
if key == v {
return true
}
Expand All @@ -106,3 +106,17 @@ func filterSystemLabels(lbs map[string]string) map[string]string {
}
return result
}

// Checks if labels array contains system labels
func ContainsSystemLabels(lbs map[string]string) bool {
for k := range lbs {
if isSystemLabel(k) {
return true
}
}
return false
}

func GetSystemLabels() []string {
return systemLabels
}
39 changes: 39 additions & 0 deletions pkg/storage/driver/util_test.go
Expand Up @@ -18,6 +18,12 @@ import (
"testing"
)

func TestGetSystemLabel(t *testing.T) {
if output := GetSystemLabels(); !reflect.DeepEqual(systemLabels, output) {
t.Errorf("Expected {%v}, got {%v}", systemLabels, output)
}
}

func TestIsSystemLabel(t *testing.T) {
tests := map[string]bool{
"name": true,
Expand Down Expand Up @@ -67,3 +73,36 @@ func TestFilterSystemLabels(t *testing.T) {
}
}
}

func TestContainsSystemLabels(t *testing.T) {
var tests = []struct {
input map[string]string
output bool
}{
{nil, false},
{map[string]string{}, false},
{map[string]string{
"name": "name",
"owner": "owner",
"status": "status",
"version": "version",
"createdAt": "createdAt",
"modifiedAt": "modifiedAt",
}, true},
{map[string]string{
"StaTus": "status",
"name": "name",
"owner": "owner",
"key": "value",
}, true},
{map[string]string{
"key1": "value1",
"key2": "value2",
}, false},
}
for _, test := range tests {
if output := ContainsSystemLabels(test.input); !reflect.DeepEqual(test.output, output) {
t.Errorf("Expected {%v}, got {%v}", test.output, output)
}
}
}

0 comments on commit 7b13ac9

Please sign in to comment.