Skip to content

Commit

Permalink
Immutable field and validation
Browse files Browse the repository at this point in the history
  • Loading branch information
@wojtek-t
wojtek-t committed Jan 12, 2020
1 parent ef69bc9 commit e612ebf
Show file tree
Hide file tree
Showing 11 changed files with 322 additions and 39 deletions.
12 changes: 12 additions & 0 deletions pkg/apis/core/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4735,6 +4735,12 @@ type Secret struct {
// +optional
metav1.ObjectMeta

// Immutable field, if set, ensures that data stored in the Secret cannot
// be updated (only object metadata can be modified).
// This is an alpha field enabled by ImmutableEphemeralVolumes feature gate.
// +optional
Immutable *bool

// Data contains the secret data. Each key must consist of alphanumeric
// characters, '-', '_' or '.'. The serialized form of the secret data is a
// base64 encoded string, representing the arbitrary (possibly non-string)
Expand Down Expand Up @@ -4857,6 +4863,12 @@ type ConfigMap struct {
// +optional
metav1.ObjectMeta

// 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.
// +optional
Immutable *bool

// 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.
Expand Down
24 changes: 23 additions & 1 deletion pkg/apis/core/validation/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
if !reflect.DeepEqual(newSecret.Data, oldSecret.Data) {
allErrs = append(allErrs, field.Forbidden(field.NewPath("data"), "field is immutable when `immutable` is set"))
}
// We don't validate StringData, as it was already converted back to Data
// before validation is happening.
}

allErrs = append(allErrs, ValidateSecret(newSecret)...)
return allErrs
Expand Down Expand Up @@ -5051,8 +5061,20 @@ func ValidateConfigMap(cfg *core.ConfigMap) field.ErrorList {
func ValidateConfigMapUpdate(newCfg, oldCfg *core.ConfigMap) field.ErrorList {
allErrs := field.ErrorList{}
allErrs = append(allErrs, ValidateObjectMetaUpdate(&newCfg.ObjectMeta, &oldCfg.ObjectMeta, field.NewPath("metadata"))...)
allErrs = append(allErrs, ValidateConfigMap(newCfg)...)

if oldCfg.Immutable != nil && *oldCfg.Immutable {
if !reflect.DeepEqual(newCfg.Immutable, oldCfg.Immutable) {
allErrs = append(allErrs, field.Forbidden(field.NewPath("immutable"), "field is immutable when `immutable` is set"))
}
if !reflect.DeepEqual(newCfg.Data, oldCfg.Data) {
allErrs = append(allErrs, field.Forbidden(field.NewPath("data"), "field is immutable when `immutable` is set"))
}
if !reflect.DeepEqual(newCfg.BinaryData, oldCfg.BinaryData) {
allErrs = append(allErrs, field.Forbidden(field.NewPath("binaryData"), "field is immutable when `immutable` is set"))
}
}

allErrs = append(allErrs, ValidateConfigMap(newCfg)...)
return allErrs
}

Expand Down
209 changes: 186 additions & 23 deletions pkg/apis/core/validation/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13117,6 +13117,104 @@ func TestValidateSecret(t *testing.T) {
}
}

func TestValidateSecretUpdate(t *testing.T) {
validSecret := func() core.Secret {
return core.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
ResourceVersion: "20",
},
Data: map[string][]byte{
"data-1": []byte("bar"),
},
}
}

falseVal := false
trueVal := true

secret := validSecret()
immutableSecret := validSecret()
immutableSecret.Immutable = &trueVal
mutableSecret := validSecret()
mutableSecret.Immutable = &falseVal

secretWithData := validSecret()
secretWithData.Data["data-2"] = []byte("baz")
immutableSecretWithData := validSecret()
immutableSecretWithData.Immutable = &trueVal
immutableSecretWithData.Data["data-2"] = []byte("baz")

secretWithChangedData := validSecret()
secretWithChangedData.Data["data-1"] = []byte("foo")
immutableSecretWithChangedData := validSecret()
immutableSecretWithChangedData.Immutable = &trueVal
immutableSecretWithChangedData.Data["data-1"] = []byte("foo")

tests := []struct {
name string
oldSecret core.Secret
newSecret core.Secret
valid bool
}{
{
name: "mark secret immutable",
oldSecret: secret,
newSecret: immutableSecret,
valid: true,
},
{
name: "revert immutable secret",
oldSecret: immutableSecret,
newSecret: secret,
valid: false,
},
{
name: "makr immutable secret mutable",
oldSecret: immutableSecret,
newSecret: mutableSecret,
valid: false,
},
{
name: "add data in secret",
oldSecret: secret,
newSecret: secretWithData,
valid: true,
},
{
name: "add data in immutable secret",
oldSecret: immutableSecret,
newSecret: immutableSecretWithData,
valid: false,
},
{
name: "change data in secret",
oldSecret: secret,
newSecret: secretWithChangedData,
valid: true,
},
{
name: "change data in immutable secret",
oldSecret: immutableSecret,
newSecret: immutableSecretWithChangedData,
valid: false,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
errs := ValidateSecretUpdate(&tc.newSecret, &tc.oldSecret)
if tc.valid && len(errs) > 0 {
t.Errorf("Unexpected error: %v", errs)
}
if !tc.valid && len(errs) == 0 {
t.Errorf("Unexpected lack of error")
}
})
}
}

func TestValidateDockerConfigSecret(t *testing.T) {
validDockerSecret := func() core.Secret {
return core.Secret{
Expand Down Expand Up @@ -13731,40 +13829,105 @@ func TestValidateConfigMapUpdate(t *testing.T) {
Data: data,
}
}
validConfigMap := func() core.ConfigMap {
return newConfigMap("1", "validname", "validdns", map[string]string{"key": "value"})
}

var (
validConfigMap = newConfigMap("1", "validname", "validns", map[string]string{"key": "value"})
noVersion = newConfigMap("", "validname", "validns", map[string]string{"key": "value"})
)
falseVal := false
trueVal := true

configMap := validConfigMap()
immutableConfigMap := validConfigMap()
immutableConfigMap.Immutable = &trueVal
mutableConfigMap := validConfigMap()
mutableConfigMap.Immutable = &falseVal

configMapWithData := validConfigMap()
configMapWithData.Data["key-2"] = "value-2"
immutableConfigMapWithData := validConfigMap()
immutableConfigMapWithData.Immutable = &trueVal
immutableConfigMapWithData.Data["key-2"] = "value-2"

configMapWithChangedData := validConfigMap()
configMapWithChangedData.Data["key"] = "foo"
immutableConfigMapWithChangedData := validConfigMap()
immutableConfigMapWithChangedData.Immutable = &trueVal
immutableConfigMapWithChangedData.Data["key"] = "foo"

noVersion := newConfigMap("", "validname", "validns", map[string]string{"key": "value"})

cases := []struct {
name string
newCfg core.ConfigMap
oldCfg core.ConfigMap
isValid bool
name string
newCfg core.ConfigMap
oldCfg core.ConfigMap
valid bool
}{
{
name: "valid",
newCfg: validConfigMap,
oldCfg: validConfigMap,
isValid: true,
name: "valid",
newCfg: configMap,
oldCfg: configMap,
valid: true,
},
{
name: "invalid",
newCfg: noVersion,
oldCfg: validConfigMap,
isValid: false,
name: "invalid",
newCfg: noVersion,
oldCfg: configMap,
valid: false,
},
{
name: "mark configmap immutable",
oldCfg: configMap,
newCfg: immutableConfigMap,
valid: true,
},
{
name: "revert immutable configmap",
oldCfg: immutableConfigMap,
newCfg: configMap,
valid: false,
},
{
name: "mark immutable configmap mutable",
oldCfg: immutableConfigMap,
newCfg: mutableConfigMap,
valid: false,
},
{
name: "add data in configmap",
oldCfg: configMap,
newCfg: configMapWithData,
valid: true,
},
{
name: "add data in immutable configmap",
oldCfg: immutableConfigMap,
newCfg: immutableConfigMapWithData,
valid: false,
},
{
name: "change data in configmap",
oldCfg: configMap,
newCfg: configMapWithChangedData,
valid: true,
},
{
name: "change data in immutable configmap",
oldCfg: immutableConfigMap,
newCfg: immutableConfigMapWithChangedData,
valid: false,
},
}

for _, tc := range cases {
errs := ValidateConfigMapUpdate(&tc.newCfg, &tc.oldCfg)
if tc.isValid && len(errs) > 0 {
t.Errorf("%v: unexpected error: %v", tc.name, errs)
}
if !tc.isValid && len(errs) == 0 {
t.Errorf("%v: unexpected non-error", tc.name)
}
t.Run(tc.name, func(t *testing.T) {
errs := ValidateConfigMapUpdate(&tc.newCfg, &tc.oldCfg)
if tc.valid && len(errs) > 0 {
t.Errorf("Unexpected error: %v", errs)
}
if !tc.valid && len(errs) == 0 {
t.Errorf("Unexpected lack of error")
}
})
}
}

Expand Down
7 changes: 7 additions & 0 deletions pkg/features/kube_features.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,12 @@ const (
//
// Enables topology aware service routing
ServiceTopology featuregate.Feature = "ServiceTopology"

// owner: @wojtek-t
// alpha: v1.18
//
// Enables a feature to make secrets and configmaps data immutable.
ImmutableEphemeralVolumes featuregate.Feature = "ImmutableEphemeralVolumes"
)

func init() {
Expand Down Expand Up @@ -634,6 +640,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
AllowInsecureBackendProxy: {Default: true, PreRelease: featuregate.Beta},
PodDisruptionBudget: {Default: true, PreRelease: featuregate.Beta},
ServiceTopology: {Default: false, PreRelease: featuregate.Alpha},
ImmutableEphemeralVolumes: {Default: false, PreRelease: featuregate.Alpha},

// inherited features from generic apiserver, relisted here to get a conflict if it is changed
// unintentionally on either side:
Expand Down
28 changes: 21 additions & 7 deletions pkg/registry/core/configmap/strategy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@ import (
"k8s.io/apiserver/pkg/registry/rest"
pkgstorage "k8s.io/apiserver/pkg/storage"
"k8s.io/apiserver/pkg/storage/names"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/api/legacyscheme"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/apis/core/validation"
"k8s.io/kubernetes/pkg/features"
)

// strategy implements behavior for ConfigMap objects
Expand All @@ -54,7 +56,8 @@ func (strategy) NamespaceScoped() bool {
}

func (strategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
_ = obj.(*api.ConfigMap)
configMap := obj.(*api.ConfigMap)
dropDisabledFields(configMap, nil)
}

func (strategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
Expand All @@ -72,12 +75,9 @@ func (strategy) AllowCreateOnUpdate() bool {
}

func (strategy) PrepareForUpdate(ctx context.Context, newObj, oldObj runtime.Object) {
_ = oldObj.(*api.ConfigMap)
_ = newObj.(*api.ConfigMap)
}

func (strategy) AllowUnconditionalUpdate() bool {
return true
oldConfigMap := oldObj.(*api.ConfigMap)
newConfigMap := newObj.(*api.ConfigMap)
dropDisabledFields(newConfigMap, oldConfigMap)
}

func (strategy) ValidateUpdate(ctx context.Context, newObj, oldObj runtime.Object) field.ErrorList {
Expand All @@ -86,6 +86,20 @@ func (strategy) ValidateUpdate(ctx context.Context, newObj, oldObj runtime.Objec
return validation.ValidateConfigMapUpdate(newCfg, oldCfg)
}

func isImmutableInUse(configMap *api.ConfigMap) bool {
return configMap != nil && configMap.Immutable != nil
}

func dropDisabledFields(configMap *api.ConfigMap, oldConfigMap *api.ConfigMap) {
if !utilfeature.DefaultFeatureGate.Enabled(features.ImmutableEphemeralVolumes) && !isImmutableInUse(oldConfigMap) {
configMap.Immutable = nil
}
}

func (strategy) AllowUnconditionalUpdate() bool {
return true
}

// GetAttrs returns labels and fields of a given object for filtering purposes.
func GetAttrs(obj runtime.Object) (labels.Set, fields.Set, error) {
configMap, ok := obj.(*api.ConfigMap)
Expand Down
Loading

0 comments on commit e612ebf

Please sign in to comment.