API for immutable Secrets and ConfigMaps #86377

wojtek-t · 2019-12-18T08:42:42Z

This implements the API part of the KEP (allow for marking secrets/configmaps as immutable + necessary bits to enforce that and e2e tests for it).

The Kubelet part is not yet implemented (those this part itself solve the first user problem being ability to protect from accidental bad pushes).

Part of kubernetes/enhancements#1412

Introduce Alpha field `Immutable` in both Secret and ConfigMap objects to mark their contents as immutable. The implementation is hidden behind feature gate `ImmutableEphemeralVolumes` (currently in Alpha stage).

fejta-bot · 2019-12-18T11:56:56Z

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

sftim · 2019-12-19T17:21:30Z

api/openapi-spec/swagger.json

@@ -5354,6 +5354,10 @@
          "description": "Data contains the configuration data. Each key must consist of alphanumeric characters, '-', '_' or '.'. Values with non-UTF-8 byte sequences must use the BinaryData field. The keys stored in Data must not overlap with the keys in the BinaryData field, this is enforced during validation process.",
          "type": "object"
        },
+        "immutable": {
+          "description": "Immutable field, if set, ensures that data stored in the configmap cannot be updated (only object metadata can be modified). This is an alpha field enabled by ImmutableEphemeralVolumes feature gate.",


nit, capitalization

Suggested change

"description": "Immutable field, if set, ensures that data stored in the configmap cannot be updated (only object metadata can be modified). This is an alpha field enabled by ImmutableEphemeralVolumes feature gate.",

"description": "Immutable field, if set, ensures that data stored in the ConfigMap cannot be updated (only object metadata can be modified). This is an alpha field enabled by ImmutableEphemeralVolumes feature gate.",

saad-ali

The Kubelet part is not yet implemented (those this part itself solve the first user problem being ability to protect from accidental bad pushes).

Is the plan to get the kubelet changes in this release as well?

pkg/apis/core/validation/validation.go

pkg/registry/core/configmap/strategy.go

pkg/apis/core/validation/validation.go

wojtek-t · 2020-01-06T15:24:58Z

Is the plan to get the kubelet changes in this release as well?

Yes - I have them as part of my POC - just need to clean them up.
That said, I think this PR is self-contained, so I was thinking about doing them as a separate PR.

Re other comments - thanks, will address them.

wojtek-t

@saad-ali - thanks for the review.
Comments addressed or responded to. PTAL

pkg/apis/core/validation/validation.go

saad-ali

/lgtm

wojtek-t · 2020-01-08T08:04:02Z

Thanks @saad-ali !

wojtek-t · 2020-01-08T08:04:49Z

/assign @thockin
For API approval (already reviewed by Saad (api-reviewer)).

thockin · 2020-01-10T18:15:05Z

staging/src/k8s.io/api/core/v1/types.go

@@ -5424,6 +5424,12 @@ type Secret struct {
 	// +optional
 	metav1.ObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"`

+	// Immutable field, if set, ensures that data stored in the Secret cannot


Since comments become docs...

"Immutable ensures that data ..." (basically drop 'field')

Document the default?

Is this field updatable or only set at creation?

Can this field change from false to true?

Addressed all (here and in configmap).

thockin · 2020-01-10T18:21:16Z

pkg/registry/core/secret/strategy.go

 }

 func (strategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
 	return validation.ValidateSecretUpdate(obj.(*api.Secret), old.(*api.Secret))
 }

+func dropDisabledFields(secret *api.Secret, oldSecret *api.Secret) {
+	if !utilfeature.DefaultFeatureGate.Enabled(features.ImmutableEphemeralVolumes) && oldSecret == nil {


Most times I see this call an "is in use" funtion that encapsulates both old == nil and old.field == nil.

pkg/registry/core/configmap/strategy.go

pkg/apis/core/validation/validation.go

thockin · 2020-01-10T18:26:19Z

pkg/apis/core/validation/validation.go

@@ -5005,6 +5005,16 @@ func ValidateSecretUpdate(newSecret, oldSecret *core.Secret) field.ErrorList {
 	}

 	allErrs = append(allErrs, ValidateImmutableField(newSecret.Type, oldSecret.Type, field.NewPath("type"))...)
+	if oldSecret.Immutable != nil && *oldSecret.Immutable {
+		if !reflect.DeepEqual(newSecret.Immutable, oldSecret.Immutable) {
+			allErrs = append(allErrs, field.Forbidden(field.NewPath("immutable"), "field is immutable when immutable is set"))


Agree with @wojtek-t

field names in errors should be back-quoted: "when immutable is set"

wojtek-t

@thockin - comments addressed; PTAL

pkg/apis/core/validation/validation.go

wojtek-t · 2020-01-12T19:39:26Z

pkg/apis/core/validation/validation.go

@@ -5005,6 +5005,16 @@ func ValidateSecretUpdate(newSecret, oldSecret *core.Secret) field.ErrorList {
 	}

 	allErrs = append(allErrs, ValidateImmutableField(newSecret.Type, oldSecret.Type, field.NewPath("type"))...)
+	if oldSecret.Immutable != nil && *oldSecret.Immutable {
+		if !reflect.DeepEqual(newSecret.Immutable, oldSecret.Immutable) {
+			allErrs = append(allErrs, field.Forbidden(field.NewPath("immutable"), "field is immutable when immutable is set"))


Fixed (both here, below and for configmap)

wojtek-t · 2020-01-12T19:46:05Z

pkg/registry/core/secret/strategy.go

 }

 func (strategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
 	return validation.ValidateSecretUpdate(obj.(*api.Secret), old.(*api.Secret))
 }

+func dropDisabledFields(secret *api.Secret, oldSecret *api.Secret) {
+	if !utilfeature.DefaultFeatureGate.Enabled(features.ImmutableEphemeralVolumes) && oldSecret == nil {


done (both here and in configmap)

wojtek-t · 2020-01-12T20:12:06Z

staging/src/k8s.io/api/core/v1/types.go

@@ -5424,6 +5424,12 @@ type Secret struct {
 	// +optional
 	metav1.ObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"`

+	// Immutable field, if set, ensures that data stored in the Secret cannot


Addressed all (here and in configmap).

wojtek-t · 2020-01-13T10:10:19Z

/retest

wojtek-t · 2020-01-13T12:08:32Z

/retest

wojtek-t · 2020-01-13T13:06:07Z

/retest

wojtek-t · 2020-01-14T13:42:52Z

@thockin - this is now ready for the second pass (test failures are unrelated flakes)

/retest

thockin

Last bit can be a followup

Thanks!

/lgtm
/approve

k8s-ci-robot · 2020-01-18T00:49:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: thockin, wojtek-t

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~api/OWNERS~~ [thockin]
~~pkg/apis/core/OWNERS~~ [thockin,wojtek-t]
~~pkg/features/OWNERS~~ [thockin,wojtek-t]
~~pkg/registry/OWNERS~~ [thockin,wojtek-t]
~~staging/src/k8s.io/api/OWNERS~~ [thockin]
~~staging/src/k8s.io/cli-runtime/OWNERS~~ [thockin,wojtek-t]
~~staging/src/k8s.io/kubectl/OWNERS~~ [thockin,wojtek-t]
~~test/e2e/common/OWNERS~~ [thockin,wojtek-t]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

fejta-bot · 2020-01-18T02:52:28Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2020-01-18T06:01:28Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2020-01-18T11:16:27Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

Fixed typo. related PR: kubernetes/kubernetes#86377

API for immutable Secrets and ConfigMaps Kubernetes-commit: 37d9c22

wojtek-t assigned saad-ali Dec 18, 2019

k8s-ci-robot requested review from brendandburns and deads2k December 18, 2019 08:43

wojtek-t force-pushed the immutable_secrets_api branch from 80e0e76 to 12c10ea Compare December 18, 2019 10:11

wojtek-t changed the title ~~[WIP] API for immutable Secrets and ConfigMaps~~ API for immutable Secrets and ConfigMaps Dec 18, 2019

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 18, 2019

sftim reviewed Dec 19, 2019

View reviewed changes

wojtek-t force-pushed the immutable_secrets_api branch from 12c10ea to 333845a Compare December 20, 2019 08:27

saad-ali reviewed Jan 4, 2020

View reviewed changes

pkg/apis/core/validation/validation.go Show resolved Hide resolved

pkg/registry/core/configmap/strategy.go Outdated Show resolved Hide resolved

pkg/apis/core/validation/validation.go Show resolved Hide resolved

wojtek-t force-pushed the immutable_secrets_api branch from 333845a to 79dd940 Compare January 7, 2020 10:31

wojtek-t commented Jan 7, 2020

View reviewed changes

pkg/apis/core/validation/validation.go Show resolved Hide resolved

pkg/apis/core/validation/validation.go Show resolved Hide resolved

wojtek-t added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jan 7, 2020

saad-ali approved these changes Jan 8, 2020

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 8, 2020

k8s-ci-robot assigned thockin Jan 8, 2020

thockin reviewed Jan 10, 2020

View reviewed changes

wojtek-t added 3 commits January 12, 2020 20:51

Immutable field and validation

e612ebf

Autogenerated

9174905

Immutable secrets/configmaps tests

7cc3971

wojtek-t commented Jan 12, 2020

View reviewed changes

wojtek-t force-pushed the immutable_secrets_api branch from 79dd940 to 7cc3971 Compare January 13, 2020 09:00

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 13, 2020

thockin reviewed Jan 18, 2020

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 18, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 18, 2020

k8s-ci-robot merged commit 37d9c22 into kubernetes:master Jan 18, 2020

k8s-ci-robot added this to the v1.18 milestone Jan 18, 2020

wojtek-t mentioned this pull request Jan 28, 2020

Cleanup validation for immutable secrets/configmaps #87602

Merged

uthark added a commit to uthark/kubernetes.github.io that referenced this pull request Jul 21, 2020

Fixed typo in feature gate name.

e5e0faf

Fixed typo. related PR: kubernetes/kubernetes#86377

uthark mentioned this pull request Jul 21, 2020

Fixed typo in feature gate name. kubernetes/website#22627

Merged

wking pushed a commit to wking/kubernetes that referenced this pull request Jul 21, 2020

Merge pull request kubernetes#86377 from wojtek-t/immutable_secrets_api

6856f9b

API for immutable Secrets and ConfigMaps Kubernetes-commit: 37d9c22

jyotimahapatra mentioned this pull request Aug 21, 2023

Added Kind/ResourceScope support in APIService model #119748

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API for immutable Secrets and ConfigMaps #86377

API for immutable Secrets and ConfigMaps #86377

wojtek-t commented Dec 18, 2019

fejta-bot commented Dec 18, 2019

sftim Dec 19, 2019

wojtek-t Dec 20, 2019

saad-ali left a comment

wojtek-t commented Jan 6, 2020

wojtek-t left a comment

saad-ali left a comment

wojtek-t commented Jan 8, 2020

wojtek-t commented Jan 8, 2020

thockin Jan 10, 2020

thockin Jan 10, 2020

thockin Jan 10, 2020

thockin Jan 10, 2020

wojtek-t Jan 12, 2020

thockin Jan 10, 2020

thockin Jan 10, 2020

wojtek-t left a comment

wojtek-t Jan 12, 2020

wojtek-t Jan 12, 2020

wojtek-t Jan 12, 2020

wojtek-t commented Jan 13, 2020

wojtek-t commented Jan 13, 2020

wojtek-t commented Jan 13, 2020

wojtek-t commented Jan 14, 2020

thockin left a comment

k8s-ci-robot commented Jan 18, 2020

fejta-bot commented Jan 18, 2020

fejta-bot commented Jan 18, 2020

fejta-bot commented Jan 18, 2020

	"description": "Immutable field, if set, ensures that data stored in the configmap cannot be updated (only object metadata can be modified). This is an alpha field enabled by ImmutableEphemeralVolumes feature gate.",
	"description": "Immutable field, if set, ensures that data stored in the ConfigMap cannot be updated (only object metadata can be modified). This is an alpha field enabled by ImmutableEphemeralVolumes feature gate.",

API for immutable Secrets and ConfigMaps #86377

API for immutable Secrets and ConfigMaps #86377

Conversation

wojtek-t commented Dec 18, 2019

fejta-bot commented Dec 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saad-ali left a comment

Choose a reason for hiding this comment

wojtek-t commented Jan 6, 2020

wojtek-t left a comment

Choose a reason for hiding this comment

saad-ali left a comment

Choose a reason for hiding this comment

wojtek-t commented Jan 8, 2020

wojtek-t commented Jan 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wojtek-t left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wojtek-t commented Jan 13, 2020

wojtek-t commented Jan 13, 2020

wojtek-t commented Jan 13, 2020

wojtek-t commented Jan 14, 2020

thockin left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jan 18, 2020

fejta-bot commented Jan 18, 2020

fejta-bot commented Jan 18, 2020

fejta-bot commented Jan 18, 2020