Repository: helm/helm
Pull request #10533 (2022-01-08 02:17:41)

Adds labels support for install and upgrade commands
<!--  Thanks for sending a pull request!  Here are some tips for you:
1. Make sure to read the Contributing Guide before submitting your PR: https://github.com/helm/helm/blob/main/CONTRIBUTING.md
2. If this PR closes another issue, add 'closes #<issue number>' somewhere in the PR summary. GitHub will automatically close that issue when this PR gets merged. Alternatively, adding 'refs #<issue number>' will not close the issue, but help provide the reviewer more context.-->
/closes https://github.com/helm/helm/issues/7007

**What this PR does / why we need it**:
Adding support of providing custom labels during install/update of release that could be used for selector filtering introduced in https://github.com/helm/helm/pull/8532.


**Special notes for your reviewer**:
This PR focused on adding custom label support during install/upgrade actions.
There was similar PRs before that, unfortunately, haven't been finished: https://github.com/helm/helm/pull/8546 https://github.com/helm/helm/pull/8099 https://github.com/helm/helm/pull/9087

In this PR I'm trying to make changes as small as possible, to make development and review easier.
My goal is to make selector functionality in the list command useable for install/upgrade actions with secret and configmap storage drivers.

I don't want to add SQL support in this particular PR, cause it seems it would at least double the number of changes.

TODO:
Functionality (support labels argument during following actions):
- [x] install
- [x] upgrade --install
- [x] upgrade (basic)
- [x] upgrade (keep labels on superseded releases)
- [x] tests for new functionality (don't know how to implement it better)
- [x] Secret storage driver
- [x] Configmap storage driver

Not planned in this PR:
- [x] SQL - support is not currently planned in this PR. But there are some related developments in https://github.com/helm/helm/pull/8546 that could be prepared as separate PR later.
- [x] Merge original release labels with upgrade labels

**UPD**:
Now I have implemented even features I haven't planned for this PR. So right now this PR is completely ready for review.

If you're interested why I've created a separate DB table (in SQL driver) for custom releases - my idea was that some day we may want to make release filtering on storage backend side (k8s and SQL support requests with filtering by labels/joined table). so I decided that it would be better to split it to a separate table instead of having it encoded somewhere in json, because data migration is always annoying.

**If applicable**:
- [ ] this PR contains documentation
- [x] this PR contains unit tests
- [x] this PR has been tested for backwards compatibility


dm3ch: Actually, I'm searching for help with following errors:
https://app.circleci.com/pipelines/github/helm/helm/3486/workflows/e06a9fd9-c72c-47a4-adfe-c8fca93560b2/jobs/15065

The problem is that I'm inserting multiple rows in `for r := range map[string]string` and I'm getting different orders for maps with same data in insert and test code places. Unfortunately, I haven't yet found a workaround for that, so this test flack sometime
dm3ch: @hickeyma what should I do to start the review process?

Sorry for the direct mention, but you are the person who helped me with making https://github.com/helm/helm/pull/8532 merged.
hickeyma: @dm3ch I have added to "awaiting review" label to it to notify maintainers that it needs a review. They should get to it when they have the bandwidth.
dm3ch: > @dm3ch I have added to "awaiting review" label to it to notify maintainers that it needs a review. They should get to it when they have the bandwidth.

Thank you
koulq: Hi, which version will this feature be put into
dm3ch: > Hi, which version will this feature be put into
Maintainers still haven't started review process, so I don't have even assumptions when this feature would be released
dm3ch: @hickeyma Sorry for the mention, but you are the only person who helped me previous time.

Is it normal that PR is hanging for such a long time without any reaction or I have done something wrong and need to ping core team to draw attention?
hickeyma: @dm3ch Do you mind doing a rebase to resolve the conflict?
dm3ch: > @dm3ch Do you mind doing a rebase to resolve the conflict?

Done, but as far as I remember there was one flaky test (some of SQL one), that I wasn't able to stabilize myself. So it's possible that now test CI job can fail.
dm3ch: And it failed on that particular flaky test with SQL. :)

Patch:
diff --git a/cmd/helm/install.go b/cmd/helm/install.go
index 7d1a761f8..ed949fdd7 100644
--- a/cmd/helm/install.go
+++ b/cmd/helm/install.go
@@ -189,6 +189,7 @@ func addInstallFlags(cmd *cobra.Command, f *pflag.FlagSet, client *action.Instal
 	f.BoolVar(&client.Atomic, "atomic", false, "if set, the installation process deletes the installation on failure. The --wait flag will be set automatically if --atomic is used")
 	f.BoolVar(&client.SkipCRDs, "skip-crds", false, "if set, no CRDs will be installed. By default, CRDs are installed if not already present")
 	f.BoolVar(&client.SubNotes, "render-subchart-notes", false, "if set, render subchart notes along with the parent")
+	f.StringToStringVarP(&client.Labels, "labels", "l", nil, "Labels that would be added to release metadata. Should be divided by comma.")
 	f.BoolVar(&client.EnableDNS, "enable-dns", false, "enable DNS lookups when rendering templates")
 	addValueOptionsFlags(f, valueOpts)
 	addChartPathOptionsFlags(f, &client.ChartPathOptions)
diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go
index 1eaa2e350..886f40292 100644
--- a/cmd/helm/upgrade.go
+++ b/cmd/helm/upgrade.go
@@ -140,6 +140,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 					instClient.SubNotes = client.SubNotes
 					instClient.Description = client.Description
 					instClient.DependencyUpdate = client.DependencyUpdate
+					instClient.Labels = client.Labels
 					instClient.EnableDNS = client.EnableDNS
 
 					rel, err := runInstall(args, instClient, valueOpts, out)
@@ -257,6 +258,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	f.IntVar(&client.MaxHistory, "history-max", settings.MaxHistory, "limit the maximum number of revisions saved per release. Use 0 for no limit")
 	f.BoolVar(&client.CleanupOnFail, "cleanup-on-fail", false, "allow deletion of new resources created in this upgrade when upgrade fails")
 	f.BoolVar(&client.SubNotes, "render-subchart-notes", false, "if set, render subchart notes along with the parent")
+	f.StringToStringVarP(&client.Labels, "labels", "l", nil, "Labels that would be added to release metadata. Should be separated by comma. Original release labels will be merged with upgrade labels. You can unset label using null.")
 	f.StringVar(&client.Description, "description", "", "add a custom description")
 	f.BoolVar(&client.DependencyUpdate, "dependency-update", false, "update dependencies if they are missing before installing the chart")
 	f.BoolVar(&client.EnableDNS, "enable-dns", false, "enable DNS lookups when rendering templates")
diff --git a/pkg/action/install.go b/pkg/action/install.go
index 11fdc4374..a5026266d 100644
--- a/pkg/action/install.go
+++ b/pkg/action/install.go
@@ -92,6 +92,7 @@ type Install struct {
 	SubNotes                 bool
 	DisableOpenAPIValidation bool
 	IncludeCRDs              bool
+	Labels                   map[string]string
 	// KubeVersion allows specifying a custom kubernetes version to use and
 	// APIVersions allows a manual set of supported API Versions to be passed
 	// (for things like templating). These are ignored if ClientOnly is false
@@ -290,7 +291,11 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma
 		return nil, err
 	}
 
-	rel := i.createRelease(chrt, vals)
+	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
 	rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer, interactWithRemote, i.EnableDNS)
@@ -534,7 +539,7 @@ func (i *Install) availableName() error {
 }
 
 // createRelease creates a new release object
-func (i *Install) createRelease(chrt *chart.Chart, rawVals map[string]interface{}) *release.Release {
+func (i *Install) createRelease(chrt *chart.Chart, rawVals map[string]interface{}, labels map[string]string) *release.Release {
 	ts := i.cfg.Now()
 	return &release.Release{
 		Name:      i.ReleaseName,
@@ -547,6 +552,7 @@ func (i *Install) createRelease(chrt *chart.Chart, rawVals map[string]interface{
 			Status:        release.StatusUnknown,
 		},
 		Version: 1,
+		Labels:  labels,
 	}
 }
 
diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go
index ebe3dd2ee..9c837dc60 100644
--- a/pkg/action/upgrade.go
+++ b/pkg/action/upgrade.go
@@ -94,6 +94,7 @@ type Upgrade struct {
 	SubNotes bool
 	// Description is the description of this operation
 	Description string
+	Labels      map[string]string
 	// PostRender is an optional post-renderer
 	//
 	// If this is non-nil, then after templates are rendered, they will be sent to the
@@ -261,6 +262,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,
@@ -276,6 +282,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin
 		Version:  revision,
 		Manifest: manifestDoc.String(),
 		Hooks:    hooks,
+		Labels:   mergeCustomLabels(lastRelease.Labels, u.Labels),
 	}
 
 	if len(notesTxt) > 0 {
@@ -598,3 +605,13 @@ func objectKey(r *resource.Info) string {
 	gvk := r.Object.GetObjectKind().GroupVersionKind()
 	return fmt.Sprintf("%s/%s/%s/%s", gvk.GroupVersion().String(), gvk.Kind, r.Namespace, r.Name)
 }
+
+func mergeCustomLabels(current, desired map[string]string) map[string]string {
+	labels := mergeStrStrMaps(current, desired)
+	for k, v := range labels {
+		if v == "null" {
+			delete(labels, k)
+		}
+	}
+	return labels
+}
diff --git a/pkg/storage/driver/cfgmaps.go b/pkg/storage/driver/cfgmaps.go
index a63fec011..0f3ec38a0 100644
--- a/pkg/storage/driver/cfgmaps.go
+++ b/pkg/storage/driver/cfgmaps.go
@@ -78,6 +78,7 @@ func (cfgmaps *ConfigMaps) Get(key string) (*rspb.Release, error) {
 		cfgmaps.Log("get: failed to decode data %q: %s", key, err)
 		return nil, err
 	}
+	r.Labels = filterSystemLabels(obj.ObjectMeta.Labels)
 	// return the release object
 	return r, nil
 }
@@ -106,7 +107,7 @@ func (cfgmaps *ConfigMaps) List(filter func(*rspb.Release) bool) ([]*rspb.Releas
 			continue
 		}
 
-		rls.Labels = item.ObjectMeta.Labels
+		rls.Labels = filterSystemLabels(item.ObjectMeta.Labels)
 
 		if filter(rls) {
 			results = append(results, rls)
@@ -145,6 +146,7 @@ func (cfgmaps *ConfigMaps) Query(labels map[string]string) ([]*rspb.Release, err
 			cfgmaps.Log("query: failed to decode release: %s", err)
 			continue
 		}
+		rls.Labels = filterSystemLabels(item.ObjectMeta.Labels)
 		results = append(results, rls)
 	}
 	return results, nil
@@ -157,6 +159,7 @@ func (cfgmaps *ConfigMaps) Create(key string, rls *rspb.Release) error {
 	var lbs labels
 
 	lbs.init()
+	lbs.fromMap(rls.Labels)
 	lbs.set("createdAt", strconv.Itoa(int(time.Now().Unix())))
 
 	// create a new configmap to hold the release
@@ -184,6 +187,7 @@ func (cfgmaps *ConfigMaps) Update(key string, rls *rspb.Release) error {
 	var lbs labels
 
 	lbs.init()
+	lbs.fromMap(rls.Labels)
 	lbs.set("modifiedAt", strconv.Itoa(int(time.Now().Unix())))
 
 	// create a new configmap object to hold the release
@@ -239,6 +243,9 @@ func newConfigMapsObject(key string, rls *rspb.Release, lbs labels) (*v1.ConfigM
 		lbs.init()
 	}
 
+	// apply custom labels
+	lbs.fromMap(rls.Labels)
+
 	// apply labels
 	lbs.set("name", rls.Name)
 	lbs.set("owner", owner)
diff --git a/pkg/storage/driver/secrets.go b/pkg/storage/driver/secrets.go
index 56df54040..224026b07 100644
--- a/pkg/storage/driver/secrets.go
+++ b/pkg/storage/driver/secrets.go
@@ -72,6 +72,7 @@ func (secrets *Secrets) Get(key string) (*rspb.Release, error) {
 	}
 	// found the secret, decode the base64 data string
 	r, err := decodeRelease(string(obj.Data["release"]))
+	r.Labels = filterSystemLabels(obj.ObjectMeta.Labels)
 	return r, errors.Wrapf(err, "get: failed to decode data %q", key)
 }
 
@@ -98,7 +99,7 @@ func (secrets *Secrets) List(filter func(*rspb.Release) bool) ([]*rspb.Release,
 			continue
 		}
 
-		rls.Labels = item.ObjectMeta.Labels
+		rls.Labels = filterSystemLabels(item.ObjectMeta.Labels)
 
 		if filter(rls) {
 			results = append(results, rls)
@@ -136,6 +137,7 @@ func (secrets *Secrets) Query(labels map[string]string) ([]*rspb.Release, error)
 			secrets.Log("query: failed to decode release: %s", err)
 			continue
 		}
+		rls.Labels = filterSystemLabels(item.ObjectMeta.Labels)
 		results = append(results, rls)
 	}
 	return results, nil
@@ -148,6 +150,7 @@ func (secrets *Secrets) Create(key string, rls *rspb.Release) error {
 	var lbs labels
 
 	lbs.init()
+	lbs.fromMap(rls.Labels)
 	lbs.set("createdAt", strconv.Itoa(int(time.Now().Unix())))
 
 	// create a new secret to hold the release
@@ -173,6 +176,7 @@ func (secrets *Secrets) Update(key string, rls *rspb.Release) error {
 	var lbs labels
 
 	lbs.init()
+	lbs.fromMap(rls.Labels)
 	lbs.set("modifiedAt", strconv.Itoa(int(time.Now().Unix())))
 
 	// create a new secret object to hold the release
@@ -221,6 +225,9 @@ func newSecretsObject(key string, rls *rspb.Release, lbs labels) (*v1.Secret, er
 		lbs.init()
 	}
 
+	// apply custom labels
+	lbs.fromMap(rls.Labels)
+
 	// apply labels
 	lbs.set("name", rls.Name)
 	lbs.set("owner", owner)
diff --git a/pkg/storage/driver/sql.go b/pkg/storage/driver/sql.go
index c8a6ae04f..18f51f3fd 100644
--- a/pkg/storage/driver/sql.go
+++ b/pkg/storage/driver/sql.go
@@ -49,6 +49,7 @@ const postgreSQLDialect = "postgres"
 const SQLDriverName = "SQL"
 
 const sqlReleaseTableName = "releases_v1"
+const sqlCustomLabelsTableName = "custom_labels_v1"
 
 const (
 	sqlReleaseTableKeyColumn        = "key"
@@ -61,6 +62,17 @@ const (
 	sqlReleaseTableOwnerColumn      = "owner"
 	sqlReleaseTableCreatedAtColumn  = "createdAt"
 	sqlReleaseTableModifiedAtColumn = "modifiedAt"
+
+	sqlCustomLabelsTableReleaseKeyColumn       = "releaseKey"
+	sqlCustomLabelsTableReleaseNamespaceColumn = "releaseNamespace"
+	sqlCustomLabelsTableKeyColumn              = "key"
+	sqlCustomLabelsTableValueColumn            = "value"
+)
+
+// Following limits based on k8s labels limits - https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set
+const (
+	sqlCustomLabelsTableKeyMaxLenght   = 253 + 1 + 63
+	sqlCustomLabelsTableValueMaxLenght = 63
 )
 
 const (
@@ -150,6 +162,41 @@ func (s *SQL) ensureDBSetup() error {
 					`, sqlReleaseTableName),
 				},
 			},
+			{
+				Id: "custom_labels",
+				Up: []string{
+					fmt.Sprintf(`
+						CREATE TABLE %s (
+							%s VARCHAR(64),
+							%s VARCHAR(67),
+							%s VARCHAR(%d), 
+							%s VARCHAR(%d)
+						);
+						CREATE INDEX ON %s (%s, %s);
+						
+						GRANT ALL ON %s TO PUBLIC;
+						ALTER TABLE %s ENABLE ROW LEVEL SECURITY;
+					`,
+						sqlCustomLabelsTableName,
+						sqlCustomLabelsTableReleaseKeyColumn,
+						sqlCustomLabelsTableReleaseNamespaceColumn,
+						sqlCustomLabelsTableKeyColumn,
+						sqlCustomLabelsTableKeyMaxLenght,
+						sqlCustomLabelsTableValueColumn,
+						sqlCustomLabelsTableValueMaxLenght,
+						sqlCustomLabelsTableName,
+						sqlCustomLabelsTableReleaseKeyColumn,
+						sqlCustomLabelsTableReleaseNamespaceColumn,
+						sqlCustomLabelsTableName,
+						sqlCustomLabelsTableName,
+					),
+				},
+				Down: []string{
+					fmt.Sprintf(`
+						DELETE TABLE %s;
+					`, sqlCustomLabelsTableName),
+				},
+			},
 		},
 	}
 
@@ -180,6 +227,13 @@ type SQLReleaseWrapper struct {
 	ModifiedAt int    `db:"modifiedAt"`
 }
 
+type SQLReleaseCustomLabelWrapper struct {
+	ReleaseKey       string `db:"release_key"`
+	ReleaseNamespace string `db:"release_namespace"`
+	Key              string `db:"key"`
+	Value            string `db:"value"`
+}
+
 // NewSQL initializes a new sql driver.
 func NewSQL(connectionString string, logger func(string, ...interface{}), namespace string) (*SQL, error) {
 	db, err := sqlx.Connect(postgreSQLDialect, connectionString)
@@ -230,13 +284,18 @@ func (s *SQL) Get(key string) (*rspb.Release, error) {
 		return nil, err
 	}
 
+	if release.Labels, err = s.getReleaseCustomLabels(key, s.namespace); err != nil {
+		s.Log("failed to get release %s/%s custom labels: %v", s.namespace, key, err)
+		return nil, err
+	}
+
 	return release, nil
 }
 
 // List returns the list of all releases such that filter(release) == true
 func (s *SQL) List(filter func(*rspb.Release) bool) ([]*rspb.Release, error) {
 	sb := s.statementBuilder.
-		Select(sqlReleaseTableBodyColumn).
+		Select(sqlReleaseTableKeyColumn, sqlReleaseTableNamespaceColumn, sqlReleaseTableBodyColumn).
 		From(sqlReleaseTableName).
 		Where(sq.Eq{sqlReleaseTableOwnerColumn: sqlReleaseDefaultOwner})
 
@@ -264,6 +323,12 @@ func (s *SQL) List(filter func(*rspb.Release) bool) ([]*rspb.Release, error) {
 			s.Log("list: failed to decode release: %v: %v", record, err)
 			continue
 		}
+
+		if release.Labels, err = s.getReleaseCustomLabels(record.Key, record.Namespace); err != nil {
+			s.Log("failed to get release %s/%s custom labels: %v", record.Namespace, record.Key, err)
+			return nil, err
+		}
+
 		if filter(release) {
 			releases = append(releases, release)
 		}
@@ -275,7 +340,7 @@ func (s *SQL) List(filter func(*rspb.Release) bool) ([]*rspb.Release, error) {
 // Query returns the set of releases that match the provided set of labels.
 func (s *SQL) Query(labels map[string]string) ([]*rspb.Release, error) {
 	sb := s.statementBuilder.
-		Select(sqlReleaseTableBodyColumn).
+		Select(sqlReleaseTableKeyColumn, sqlReleaseTableNamespaceColumn, sqlReleaseTableBodyColumn).
 		From(sqlReleaseTableName)
 
 	keys := make([]string, 0, len(labels))
@@ -321,6 +386,12 @@ func (s *SQL) Query(labels map[string]string) ([]*rspb.Release, error) {
 			s.Log("list: failed to decode release: %v: %v", record, err)
 			continue
 		}
+
+		if release.Labels, err = s.getReleaseCustomLabels(record.Key, record.Namespace); err != nil {
+			s.Log("failed to get release %s/%s custom labels: %v", record.Namespace, record.Key, err)
+			return nil, err
+		}
+
 		releases = append(releases, release)
 	}
 
@@ -403,6 +474,36 @@ func (s *SQL) Create(key string, rls *rspb.Release) error {
 		s.Log("failed to store release %s in SQL database: %v", key, err)
 		return err
 	}
+
+	// Filtering labels before insert cause in SQL storage driver system releases are stored in separate columns of release table
+	for k, v := range filterSystemLabels(rls.Labels) {
+		insertLabelsQuery, args, err := s.statementBuilder.
+			Insert(sqlCustomLabelsTableName).
+			Columns(
+				sqlCustomLabelsTableReleaseKeyColumn,
+				sqlCustomLabelsTableReleaseNamespaceColumn,
+				sqlCustomLabelsTableKeyColumn,
+				sqlCustomLabelsTableValueColumn,
+			).
+			Values(
+				key,
+				namespace,
+				k,
+				v,
+			).ToSql()
+
+		if err != nil {
+			defer transaction.Rollback()
+			s.Log("failed to build insert query: %v", err)
+			return err
+		}
+
+		if _, err := transaction.Exec(insertLabelsQuery, args...); err != nil {
+			defer transaction.Rollback()
+			s.Log("failed to write Labels: %v", err)
+			return err
+		}
+	}
 	defer transaction.Commit()
 
 	return nil
@@ -487,10 +588,56 @@ func (s *SQL) Delete(key string) (*rspb.Release, error) {
 		Where(sq.Eq{sqlReleaseTableNamespaceColumn: s.namespace}).
 		ToSql()
 	if err != nil {
-		s.Log("failed to build select query: %v", err)
+		s.Log("failed to build delete query: %v", err)
 		return nil, err
 	}
 
 	_, err = transaction.Exec(deleteQuery, args...)
+	if err != nil {
+		s.Log("failed perform delete query: %v", err)
+		return release, err
+	}
+
+	if release.Labels, err = s.getReleaseCustomLabels(key, s.namespace); err != nil {
+		s.Log("failed to get release %s/%s custom labels: %v", s.namespace, key, err)
+		return nil, err
+	}
+
+	deleteCustomLabelsQuery, args, err := s.statementBuilder.
+		Delete(sqlCustomLabelsTableName).
+		Where(sq.Eq{sqlCustomLabelsTableReleaseKeyColumn: key}).
+		Where(sq.Eq{sqlCustomLabelsTableReleaseNamespaceColumn: s.namespace}).
+		ToSql()
+
+	if err != nil {
+		s.Log("failed to build delete Labels query: %v", err)
+		return nil, err
+	}
+	_, err = transaction.Exec(deleteCustomLabelsQuery, args...)
 	return release, err
 }
+
+// Get release custom labels from database
+func (s *SQL) getReleaseCustomLabels(key string, namespace string) (map[string]string, error) {
+	query, args, err := s.statementBuilder.
+		Select(sqlCustomLabelsTableKeyColumn, sqlCustomLabelsTableValueColumn).
+		From(sqlCustomLabelsTableName).
+		Where(sq.Eq{sqlCustomLabelsTableReleaseKeyColumn: key,
+			sqlCustomLabelsTableReleaseNamespaceColumn: s.namespace}).
+		ToSql()
+	if err != nil {
+		return nil, err
+	}
+
+	var labelsList = []SQLReleaseCustomLabelWrapper{}
+	if err := s.db.Select(&labelsList, query, args...); err != nil {
+		return nil, err
+	}
+
+	labelsMap := make(map[string]string)
+	for _, i := range labelsList {
+		labelsMap[i.Key] = i.Value
+	}
+
+	return filterSystemLabels(labelsMap), nil
+}
diff --git a/pkg/storage/driver/util.go b/pkg/storage/driver/util.go
index 96a211e37..7bda5ec96 100644
--- a/pkg/storage/driver/util.go
+++ b/pkg/storage/driver/util.go
@@ -30,6 +30,8 @@ var b64 = base64.StdEncoding
 
 var magicGzip = []byte{0x1f, 0x8b, 0x08}
 
+var systemLabels = []string{"name", "owner", "status", "version", "createdAt", "modifiedAt"}
+
 // encodeRelease encodes a release returning a base64 encoded
 // gzipped string representation, or error.
 func encodeRelease(rls *rspb.Release) (string, error) {
@@ -83,3 +85,38 @@ func decodeRelease(data string) (*rspb.Release, error) {
 	}
 	return &rls, nil
 }
+
+// Checks if label is system
+func isSystemLabel(key string) bool {
+	for _, v := range GetSystemLabels() {
+		if key == v {
+			return true
+		}
+	}
+	return false
+}
+
+// Removes system labels from labels map
+func filterSystemLabels(lbs map[string]string) map[string]string {
+	result := make(map[string]string)
+	for k, v := range lbs {
+		if !isSystemLabel(k) {
+			result[k] = v
+		}
+	}
+	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
+}


Test patch:
diff --git a/cmd/helm/upgrade_test.go b/cmd/helm/upgrade_test.go
index e366f8d19..0e2e291f7 100644
--- a/cmd/helm/upgrade_test.go
+++ b/cmd/helm/upgrade_test.go
@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"reflect"
 	"strings"
 	"testing"
 
@@ -430,3 +431,31 @@ func TestUpgradeFileCompletion(t *testing.T) {
 	checkFileCompletion(t, "upgrade myrelease", true)
 	checkFileCompletion(t, "upgrade myrelease repo/chart", false)
 }
+
+func TestUpgradeInstallWithLabels(t *testing.T) {
+	releaseName := "funny-bunny-labels"
+	_, _, chartPath := prepareMockRelease(releaseName, t)
+
+	defer resetEnv()()
+
+	store := storageFixture()
+
+	expectedLabels := map[string]string{
+		"key1": "val1",
+		"key2": "val2",
+	}
+	cmd := fmt.Sprintf("upgrade %s --install --labels key1=val1,key2=val2 '%s'", releaseName, chartPath)
+	_, _, err := executeActionCommandC(store, cmd)
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	updatedRel, err := store.Get(releaseName, 1)
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	if !reflect.DeepEqual(updatedRel.Labels, expectedLabels) {
+		t.Errorf("Expected {%v}, got {%v}", expectedLabels, updatedRel.Labels)
+	}
+}
diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go
index 5e3ae79c9..d49365b05 100644
--- a/pkg/action/install_test.go
+++ b/pkg/action/install_test.go
@@ -717,3 +717,33 @@ func TestNameAndChartGenerateName(t *testing.T) {
 		})
 	}
 }
+
+func TestInstallWithLabels(t *testing.T) {
+	is := assert.New(t)
+	instAction := installAction(t)
+	instAction.Labels = map[string]string{
+		"key1": "val1",
+		"key2": "val2",
+	}
+	res, err := instAction.Run(buildChart(), nil)
+	if err != nil {
+		t.Fatalf("Failed install: %s", err)
+	}
+
+	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)
+}
diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go
index 62922b373..77656e1c5 100644
--- a/pkg/action/upgrade_test.go
+++ b/pkg/action/upgrade_test.go
@@ -19,10 +19,12 @@ package action
 import (
 	"context"
 	"fmt"
+	"reflect"
 	"testing"
 	"time"
 
 	"helm.sh/helm/v3/pkg/chart"
+	"helm.sh/helm/v3/pkg/storage/driver"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -386,5 +388,97 @@ func TestUpgradeRelease_Interrupted_Atomic(t *testing.T) {
 	is.NoError(err)
 	// Should have rolled back to the previous
 	is.Equal(updatedRes.Info.Status, release.StatusDeployed)
+}
+
+func TestMergeCustomLabels(t *testing.T) {
+	var tests = [][3]map[string]string{
+		{nil, nil, map[string]string{}},
+		{map[string]string{}, map[string]string{}, map[string]string{}},
+		{map[string]string{"k1": "v1", "k2": "v2"}, nil, map[string]string{"k1": "v1", "k2": "v2"}},
+		{nil, map[string]string{"k1": "v1", "k2": "v2"}, map[string]string{"k1": "v1", "k2": "v2"}},
+		{map[string]string{"k1": "v1", "k2": "v2"}, map[string]string{"k1": "null", "k2": "v3"}, map[string]string{"k2": "v3"}},
+	}
+	for _, test := range tests {
+		if output := mergeCustomLabels(test[0], test[1]); !reflect.DeepEqual(test[2], output) {
+			t.Errorf("Expected {%v}, got {%v}", test[2], output)
+		}
+	}
+}
+
+func TestUpgradeRelease_Labels(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",
+		"key3": "val3",
+	}
+	// setting newValues and upgrading
+	res, err := upAction.Run(rel.Name, buildChart(), nil)
+	is.NoError(err)
+
+	// Now make sure it is actually upgraded and labels were merged
+	updatedRes, err := upAction.cfg.Releases.Get(res.Name, 2)
+	is.NoError(err)
+
+	if updatedRes == nil {
+		is.Fail("Updated Release is nil")
+		return
+	}
+	is.Equal(release.StatusDeployed, updatedRes.Info.Status)
+	is.Equal(mergeCustomLabels(rel.Labels, upAction.Labels), updatedRes.Labels)
+
+	// Now make sure it is suppressed release still contains original labels
+	initialRes, err := upAction.cfg.Releases.Get(res.Name, 1)
+	is.NoError(err)
+
+	if initialRes == nil {
+		is.Fail("Updated Release is nil")
+		return
+	}
+	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)
 }
diff --git a/pkg/storage/driver/mock_test.go b/pkg/storage/driver/mock_test.go
index c0236ece8..7a1541a02 100644
--- a/pkg/storage/driver/mock_test.go
+++ b/pkg/storage/driver/mock_test.go
@@ -40,6 +40,10 @@ func releaseStub(name string, vers int, namespace string, status rspb.Status) *r
 		Version:   vers,
 		Namespace: namespace,
 		Info:      &rspb.Info{Status: status},
+		Labels: map[string]string{
+			"key1": "val1",
+			"key2": "val2",
+		},
 	}
 }
 
diff --git a/pkg/storage/driver/sql_test.go b/pkg/storage/driver/sql_test.go
index 87b6315b8..4c0c7b668 100644
--- a/pkg/storage/driver/sql_test.go
+++ b/pkg/storage/driver/sql_test.go
@@ -62,6 +62,8 @@ func TestSQLGet(t *testing.T) {
 			),
 		).RowsWillBeClosed()
 
+	mockGetReleaseCustomLabels(mock, key, namespace, rel.Labels)
+
 	got, err := sqlDriver.Get(key)
 	if err != nil {
 		t.Fatalf("Failed to get release: %v", err)
@@ -77,38 +79,42 @@ func TestSQLGet(t *testing.T) {
 }
 
 func TestSQLList(t *testing.T) {
-	body1, _ := encodeRelease(releaseStub("key-1", 1, "default", rspb.StatusUninstalled))
-	body2, _ := encodeRelease(releaseStub("key-2", 1, "default", rspb.StatusUninstalled))
-	body3, _ := encodeRelease(releaseStub("key-3", 1, "default", rspb.StatusDeployed))
-	body4, _ := encodeRelease(releaseStub("key-4", 1, "default", rspb.StatusDeployed))
-	body5, _ := encodeRelease(releaseStub("key-5", 1, "default", rspb.StatusSuperseded))
-	body6, _ := encodeRelease(releaseStub("key-6", 1, "default", rspb.StatusSuperseded))
+	releases := []*rspb.Release{}
+	releases = append(releases, releaseStub("key-1", 1, "default", rspb.StatusUninstalled))
+	releases = append(releases, releaseStub("key-2", 1, "default", rspb.StatusUninstalled))
+	releases = append(releases, releaseStub("key-3", 1, "default", rspb.StatusDeployed))
+	releases = append(releases, releaseStub("key-4", 1, "default", rspb.StatusDeployed))
+	releases = append(releases, releaseStub("key-5", 1, "default", rspb.StatusSuperseded))
+	releases = append(releases, releaseStub("key-6", 1, "default", rspb.StatusSuperseded))
 
 	sqlDriver, mock := newTestFixtureSQL(t)
 
 	for i := 0; i < 3; i++ {
 		query := fmt.Sprintf(
-			"SELECT %s FROM %s WHERE %s = $1 AND %s = $2",
+			"SELECT %s, %s, %s FROM %s WHERE %s = $1 AND %s = $2",
+			sqlReleaseTableKeyColumn,
+			sqlReleaseTableNamespaceColumn,
 			sqlReleaseTableBodyColumn,
 			sqlReleaseTableName,
 			sqlReleaseTableOwnerColumn,
 			sqlReleaseTableNamespaceColumn,
 		)
 
+		rows := mock.NewRows([]string{
+			sqlReleaseTableBodyColumn,
+		})
+		for _, r := range releases {
+			body, _ := encodeRelease(r)
+			rows.AddRow(body)
+		}
 		mock.
 			ExpectQuery(regexp.QuoteMeta(query)).
 			WithArgs(sqlReleaseDefaultOwner, sqlDriver.namespace).
-			WillReturnRows(
-				mock.NewRows([]string{
-					sqlReleaseTableBodyColumn,
-				}).
-					AddRow(body1).
-					AddRow(body2).
-					AddRow(body3).
-					AddRow(body4).
-					AddRow(body5).
-					AddRow(body6),
-			).RowsWillBeClosed()
+			WillReturnRows(rows).RowsWillBeClosed()
+
+		for _, r := range releases {
+			mockGetReleaseCustomLabels(mock, "", r.Namespace, r.Labels)
+		}
 	}
 
 	// list all deleted releases
@@ -181,6 +187,23 @@ func TestSqlCreate(t *testing.T) {
 		ExpectExec(regexp.QuoteMeta(query)).
 		WithArgs(key, sqlReleaseDefaultType, body, rel.Name, rel.Namespace, int(rel.Version), rel.Info.Status.String(), sqlReleaseDefaultOwner, int(time.Now().Unix())).
 		WillReturnResult(sqlmock.NewResult(1, 1))
+
+	labelsQuery := fmt.Sprintf(
+		"INSERT INTO %s (%s,%s,%s,%s) VALUES ($1,$2,$3,$4)",
+		sqlCustomLabelsTableName,
+		sqlCustomLabelsTableReleaseKeyColumn,
+		sqlCustomLabelsTableReleaseNamespaceColumn,
+		sqlCustomLabelsTableKeyColumn,
+		sqlCustomLabelsTableValueColumn,
+	)
+
+	mock.MatchExpectationsInOrder(false)
+	for k, v := range filterSystemLabels(rel.Labels) {
+		mock.
+			ExpectExec(regexp.QuoteMeta(labelsQuery)).
+			WithArgs(key, rel.Namespace, k, v).
+			WillReturnResult(sqlmock.NewResult(1, 1))
+	}
 	mock.ExpectCommit()
 
 	if err := sqlDriver.Create(key, rel); err != nil {
@@ -316,7 +339,9 @@ func TestSqlQuery(t *testing.T) {
 	sqlDriver, mock := newTestFixtureSQL(t)
 
 	query := fmt.Sprintf(
-		"SELECT %s FROM %s WHERE %s = $1 AND %s = $2 AND %s = $3 AND %s = $4",
+		"SELECT %s, %s, %s FROM %s WHERE %s = $1 AND %s = $2 AND %s = $3 AND %s = $4",
+		sqlReleaseTableKeyColumn,
+		sqlReleaseTableNamespaceColumn,
 		sqlReleaseTableBodyColumn,
 		sqlReleaseTableName,
 		sqlReleaseTableNameColumn,
@@ -345,8 +370,12 @@ func TestSqlQuery(t *testing.T) {
 			),
 		).RowsWillBeClosed()
 
+	mockGetReleaseCustomLabels(mock, "", deployedRelease.Namespace, deployedRelease.Labels)
+
 	query = fmt.Sprintf(
-		"SELECT %s FROM %s WHERE %s = $1 AND %s = $2 AND %s = $3",
+		"SELECT %s, %s, %s FROM %s WHERE %s = $1 AND %s = $2 AND %s = $3",
+		sqlReleaseTableKeyColumn,
+		sqlReleaseTableNamespaceColumn,
 		sqlReleaseTableBodyColumn,
 		sqlReleaseTableName,
 		sqlReleaseTableNameColumn,
@@ -367,6 +396,9 @@ func TestSqlQuery(t *testing.T) {
 			),
 		).RowsWillBeClosed()
 
+	mockGetReleaseCustomLabels(mock, "", supersededRelease.Namespace, supersededRelease.Labels)
+	mockGetReleaseCustomLabels(mock, "", deployedRelease.Namespace, deployedRelease.Labels)
+
 	_, err := sqlDriver.Query(labelSetUnknown)
 	if err == nil {
 		t.Errorf("Expected error {%v}, got nil", ErrReleaseNotFound)
@@ -447,6 +479,20 @@ func TestSqlDelete(t *testing.T) {
 		ExpectExec(regexp.QuoteMeta(deleteQuery)).
 		WithArgs(key, namespace).
 		WillReturnResult(sqlmock.NewResult(0, 1))
+
+	mockGetReleaseCustomLabels(mock, key, namespace, rel.Labels)
+
+	deleteLabelsQuery := fmt.Sprintf(
+		"DELETE FROM %s WHERE %s = $1 AND %s = $2",
+		sqlCustomLabelsTableName,
+		sqlCustomLabelsTableReleaseKeyColumn,
+		sqlCustomLabelsTableReleaseNamespaceColumn,
+	)
+	mock.
+		ExpectExec(regexp.QuoteMeta(deleteLabelsQuery)).
+		WithArgs(key, namespace).
+		WillReturnResult(sqlmock.NewResult(0, 1))
+
 	mock.ExpectCommit()
 
 	deletedRelease, err := sqlDriver.Delete(key)
@@ -461,3 +507,26 @@ func TestSqlDelete(t *testing.T) {
 		t.Errorf("Expected release {%v}, got {%v}", rel, deletedRelease)
 	}
 }
+
+func mockGetReleaseCustomLabels(mock sqlmock.Sqlmock, key string, namespace string, labels map[string]string) {
+	query := fmt.Sprintf(
+		regexp.QuoteMeta("SELECT %s, %s FROM %s WHERE %s = $1 AND %s = $2"),
+		sqlCustomLabelsTableKeyColumn,
+		sqlCustomLabelsTableValueColumn,
+		sqlCustomLabelsTableName,
+		sqlCustomLabelsTableReleaseKeyColumn,
+		sqlCustomLabelsTableReleaseNamespaceColumn,
+	)
+
+	eq := mock.ExpectQuery(query).
+		WithArgs(key, namespace)
+
+	returnRows := mock.NewRows([]string{
+		sqlCustomLabelsTableKeyColumn,
+		sqlCustomLabelsTableValueColumn,
+	})
+	for k, v := range labels {
+		returnRows.AddRow(k, v)
+	}
+	eq.WillReturnRows(returnRows).RowsWillBeClosed()
+}
diff --git a/pkg/storage/driver/util_test.go b/pkg/storage/driver/util_test.go
new file mode 100644
index 000000000..d16043924
--- /dev/null
+++ b/pkg/storage/driver/util_test.go
@@ -0,0 +1,108 @@
+/*
+Copyright The Helm Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+    http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package driver
+
+import (
+	"reflect"
+	"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,
+		"owner": true,
+		"test":  false,
+		"NaMe":  false,
+	}
+	for label, result := range tests {
+		if output := isSystemLabel(label); output != result {
+			t.Errorf("Output %t not equal to expected %t", output, result)
+		}
+	}
+}
+
+func TestFilterSystemLabels(t *testing.T) {
+	var tests = [][2]map[string]string{
+		{nil, map[string]string{}},
+		{map[string]string{}, map[string]string{}},
+		{map[string]string{
+			"name":       "name",
+			"owner":      "owner",
+			"status":     "status",
+			"version":    "version",
+			"createdAt":  "createdAt",
+			"modifiedAt": "modifiedAt",
+		}, map[string]string{}},
+		{map[string]string{
+			"StaTus": "status",
+			"name":   "name",
+			"owner":  "owner",
+			"key":    "value",
+		}, map[string]string{
+			"StaTus": "status",
+			"key":    "value",
+		}},
+		{map[string]string{
+			"key1": "value1",
+			"key2": "value2",
+		}, map[string]string{
+			"key1": "value1",
+			"key2": "value2",
+		}},
+	}
+	for _, test := range tests {
+		if output := filterSystemLabels(test[0]); !reflect.DeepEqual(test[1], output) {
+			t.Errorf("Expected {%v}, got {%v}", test[1], output)
+		}
+	}
+}
+
+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)
+		}
+	}
+}