diff --git a/pkg/apis/admissionregistration/types.go b/pkg/apis/admissionregistration/types.go index 851f9206aa3..9288057ef01 100644 --- a/pkg/apis/admissionregistration/types.go +++ b/pkg/apis/admissionregistration/types.go @@ -123,6 +123,52 @@ type ValidatingAdmissionPolicy struct { metav1.ObjectMeta // Specification of the desired behavior of the ValidatingAdmissionPolicy. Spec ValidatingAdmissionPolicySpec + // The status of the ValidatingAdmissionPolicy, including warnings that are useful to determine if the policy + // behaves in the expected way. + // Populated by the system. + // Read-only. + // +optional + Status ValidatingAdmissionPolicyStatus +} + +// ValidatingAdmissionPolicyStatus represents the status of an admission validation policy. +type ValidatingAdmissionPolicyStatus struct { + // The generation observed by the controller. + // +optional + ObservedGeneration int64 + // The results of type checking for each expression. + // Presence of this field indicates the completion of the type checking. + // +optional + TypeChecking *TypeChecking + // The conditions represent the latest available observations of a policy's current state. + // +optional + // +listType=map + // +listMapKey=type + Conditions []metav1.Condition +} + +// ValidatingAdmissionPolicyConditionType is the condition type of admission validation policy. +type ValidatingAdmissionPolicyConditionType string + +// TypeChecking contains results of type checking the expressions in the +// ValidatingAdmissionPolicy +type TypeChecking struct { + // The type checking warnings for each expression. + // +optional + // +listType=atomic + ExpressionWarnings []ExpressionWarning +} + +// ExpressionWarning is a warning information that targets a specific expression. +type ExpressionWarning struct { + // The path to the field that refers the expression. + // For example, the reference to the expression of the first item of + // validations is "spec.validations[0].expression" + FieldRef string + // The content of type checking information in a human-readable form. + // Each line of the warning contains the type that the expression is checked + // against, followed by the type check error from the compiler. + Warning string } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/admissionregistration/validation/validation.go b/pkg/apis/admissionregistration/validation/validation.go index fe9f60fa988..a3f0b459788 100644 --- a/pkg/apis/admissionregistration/validation/validation.go +++ b/pkg/apis/admissionregistration/validation/validation.go @@ -33,6 +33,7 @@ import ( celconfig "k8s.io/apiserver/pkg/apis/cel" "k8s.io/apiserver/pkg/cel" "k8s.io/apiserver/pkg/util/webhook" + "k8s.io/client-go/util/jsonpath" "k8s.io/kubernetes/pkg/apis/admissionregistration" admissionregistrationv1 "k8s.io/kubernetes/pkg/apis/admissionregistration/v1" @@ -916,7 +917,56 @@ func ValidateValidatingAdmissionPolicyUpdate(newC, oldC *admissionregistration.V return validateValidatingAdmissionPolicy(newC) } +// ValidateValidatingAdmissionPolicyStatusUpdate validates update of status of validating admission policy +func ValidateValidatingAdmissionPolicyStatusUpdate(newC, oldC *admissionregistration.ValidatingAdmissionPolicy) field.ErrorList { + return validateValidatingAdmissionPolicyStatus(&newC.Status, field.NewPath("status")) +} + // ValidateValidatingAdmissionPolicyBindingUpdate validates update of validating admission policy func ValidateValidatingAdmissionPolicyBindingUpdate(newC, oldC *admissionregistration.ValidatingAdmissionPolicyBinding) field.ErrorList { return validateValidatingAdmissionPolicyBinding(newC) } + +func validateValidatingAdmissionPolicyStatus(status *admissionregistration.ValidatingAdmissionPolicyStatus, fldPath *field.Path) field.ErrorList { + var allErrors field.ErrorList + allErrors = append(allErrors, validateTypeChecking(status.TypeChecking, fldPath.Child("typeChecking"))...) + allErrors = append(allErrors, metav1validation.ValidateConditions(status.Conditions, fldPath.Child("conditions"))...) + return allErrors +} + +func validateTypeChecking(typeChecking *admissionregistration.TypeChecking, fldPath *field.Path) field.ErrorList { + if typeChecking == nil { + return nil + } + return validateExpressionWarnings(typeChecking.ExpressionWarnings, fldPath.Child("expressionWarnings")) +} + +func validateExpressionWarnings(expressionWarnings []admissionregistration.ExpressionWarning, fldPath *field.Path) field.ErrorList { + var allErrors field.ErrorList + for i, warning := range expressionWarnings { + allErrors = append(allErrors, validateExpressionWarning(&warning, fldPath.Index(i))...) + } + return allErrors +} + +func validateExpressionWarning(expressionWarning *admissionregistration.ExpressionWarning, fldPath *field.Path) field.ErrorList { + var allErrors field.ErrorList + if expressionWarning.Warning == "" { + allErrors = append(allErrors, field.Required(fldPath.Child("warning"), "")) + } + allErrors = append(allErrors, validateFieldRef(expressionWarning.FieldRef, fldPath.Child("fieldRef"))...) + return allErrors +} + +func validateFieldRef(fieldRef string, fldPath *field.Path) field.ErrorList { + fieldRef = strings.TrimSpace(fieldRef) + if fieldRef == "" { + return field.ErrorList{field.Required(fldPath, "")} + } + jsonPath := jsonpath.New("spec") + if err := jsonPath.Parse(fmt.Sprintf("{%s}", fieldRef)); err != nil { + return field.ErrorList{field.Invalid(fldPath, fieldRef, fmt.Sprintf("invalid JSONPath: %v", err))} + } + // no further checks, for an easier upgrade/rollback + return nil +} diff --git a/pkg/apis/admissionregistration/validation/validation_test.go b/pkg/apis/admissionregistration/validation/validation_test.go index 575d7502064..8db59efa1f1 100644 --- a/pkg/apis/admissionregistration/validation/validation_test.go +++ b/pkg/apis/admissionregistration/validation/validation_test.go @@ -21,6 +21,7 @@ import ( "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/kubernetes/pkg/apis/admissionregistration" ) @@ -3453,3 +3454,83 @@ func TestValidateValidatingAdmissionPolicyBindingUpdate(t *testing.T) { } } + +func TestValidateValidatingAdmissionPolicyStatus(t *testing.T) { + for _, tc := range []struct { + name string + status *admissionregistration.ValidatingAdmissionPolicyStatus + expectedError string + }{ + { + name: "empty", + status: &admissionregistration.ValidatingAdmissionPolicyStatus{}, + }, + { + name: "type checking", + status: &admissionregistration.ValidatingAdmissionPolicyStatus{ + TypeChecking: &admissionregistration.TypeChecking{ + ExpressionWarnings: []admissionregistration.ExpressionWarning{ + { + FieldRef: "spec.validations[0].expression", + Warning: "message", + }, + }, + }, + }, + }, + { + name: "type checking bad json path", + status: &admissionregistration.ValidatingAdmissionPolicyStatus{ + TypeChecking: &admissionregistration.TypeChecking{ + ExpressionWarnings: []admissionregistration.ExpressionWarning{ + { + FieldRef: "spec[foo]", + Warning: "message", + }, + }, + }, + }, + expectedError: "invalid JSONPath: invalid array index foo", + }, + { + name: "type checking missing warning", + status: &admissionregistration.ValidatingAdmissionPolicyStatus{ + TypeChecking: &admissionregistration.TypeChecking{ + ExpressionWarnings: []admissionregistration.ExpressionWarning{ + { + FieldRef: "spec.validations[0].expression", + }, + }, + }, + }, + expectedError: "Required value", + }, + { + name: "type checking missing fieldRef", + status: &admissionregistration.ValidatingAdmissionPolicyStatus{ + TypeChecking: &admissionregistration.TypeChecking{ + ExpressionWarnings: []admissionregistration.ExpressionWarning{ + { + Warning: "message", + }, + }, + }, + }, + expectedError: "Required value", + }, + } { + t.Run(tc.name, func(t *testing.T) { + errs := validateValidatingAdmissionPolicyStatus(tc.status, field.NewPath("status")) + err := errs.ToAggregate() + if err != nil { + if e, a := tc.expectedError, err.Error(); !strings.Contains(a, e) || e == "" { + t.Errorf("expected to contain %s, got %s", e, a) + } + } else { + if tc.expectedError != "" { + t.Errorf("unexpected no error, expected to contain %s", tc.expectedError) + } + } + }) + } +} diff --git a/pkg/registry/admissionregistration/rest/storage_apiserver.go b/pkg/registry/admissionregistration/rest/storage_apiserver.go index 49dd31f028d..6752bc26eb4 100644 --- a/pkg/registry/admissionregistration/rest/storage_apiserver.go +++ b/pkg/registry/admissionregistration/rest/storage_apiserver.go @@ -95,12 +95,13 @@ func (p RESTStorageProvider) v1alpha1Storage(apiResourceConfigSource serverstora // validatingadmissionpolicies if resource := "validatingadmissionpolicies"; apiResourceConfigSource.ResourceEnabled(admissionregistrationv1alpha1.SchemeGroupVersion.WithResource(resource)) { - policyStorage, err := validatingadmissionpolicystorage.NewREST(restOptionsGetter, p.Authorizer, r) + policyStorage, policyStatusStorage, err := validatingadmissionpolicystorage.NewREST(restOptionsGetter, p.Authorizer, r) if err != nil { return storage, err } policyGetter = policyStorage storage[resource] = policyStorage + storage[resource+"/status"] = policyStatusStorage } // validatingadmissionpolicybindings diff --git a/pkg/registry/admissionregistration/validatingadmissionpolicy/storage/storage.go b/pkg/registry/admissionregistration/validatingadmissionpolicy/storage/storage.go index dfdf9339a17..c3382b65cfb 100644 --- a/pkg/registry/admissionregistration/validatingadmissionpolicy/storage/storage.go +++ b/pkg/registry/admissionregistration/validatingadmissionpolicy/storage/storage.go @@ -17,6 +17,9 @@ limitations under the License. package storage import ( + "context" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/registry/generic" @@ -28,6 +31,7 @@ import ( printerstorage "k8s.io/kubernetes/pkg/printers/storage" "k8s.io/kubernetes/pkg/registry/admissionregistration/resolver" "k8s.io/kubernetes/pkg/registry/admissionregistration/validatingadmissionpolicy" + "sigs.k8s.io/structured-merge-diff/v4/fieldpath" ) // REST implements a RESTStorage for validatingAdmissionPolicy against etcd @@ -35,10 +39,16 @@ type REST struct { *genericregistry.Store } +// StatusREST implements a RESTStorage for ValidatingAdmissionPolicyStatus +type StatusREST struct { + // DO NOT embed Store, manually select function to export. + store *genericregistry.Store +} + var groupResource = admissionregistration.Resource("validatingadmissionpolicies") -// NewREST returns a RESTStorage object that will work against validatingAdmissionPolicy. -func NewREST(optsGetter generic.RESTOptionsGetter, authorizer authorizer.Authorizer, resourceResolver resolver.ResourceResolver) (*REST, error) { +// NewREST returns two RESTStorage objects that will work against validatingAdmissionPolicy and its status. +func NewREST(optsGetter generic.RESTOptionsGetter, authorizer authorizer.Authorizer, resourceResolver resolver.ResourceResolver) (*REST, *StatusREST, error) { r := &REST{} strategy := validatingadmissionpolicy.NewStrategy(authorizer, resourceResolver) store := &genericregistry.Store{ @@ -50,18 +60,24 @@ func NewREST(optsGetter generic.RESTOptionsGetter, authorizer authorizer.Authori DefaultQualifiedResource: groupResource, SingularQualifiedResource: admissionregistration.Resource("validatingadmissionpolicy"), - CreateStrategy: strategy, - UpdateStrategy: strategy, - DeleteStrategy: strategy, + CreateStrategy: strategy, + UpdateStrategy: strategy, + DeleteStrategy: strategy, + ResetFieldsStrategy: strategy, TableConvertor: printerstorage.TableConvertor{TableGenerator: printers.NewTableGenerator().With(printersinternal.AddHandlers)}, } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - return nil, err + return nil, nil, err } r.Store = store - return r, nil + statusStrategy := validatingadmissionpolicy.NewStatusStrategy(strategy) + statusStore := *store + statusStore.UpdateStrategy = statusStrategy + statusStore.ResetFieldsStrategy = statusStrategy + sr := &StatusREST{store: &statusStore} + return r, sr, nil } // Implement CategoriesProvider @@ -71,3 +87,34 @@ var _ rest.CategoriesProvider = &REST{} func (r *REST) Categories() []string { return []string{"api-extensions"} } + +// New generates a new ValidatingAdmissionPolicy object +func (r *StatusREST) New() runtime.Object { + return &admissionregistration.ValidatingAdmissionPolicy{} +} + +// Destroy disposes the store object. For the StatusREST, this is a no-op. +func (r *StatusREST) Destroy() { + // Given that underlying store is shared with REST, + // we don't destroy it here explicitly. +} + +// Get retrieves the object from the storage. It is required to support Patch. +func (r *StatusREST) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { + return r.store.Get(ctx, name, options) +} + +// GetResetFields returns the fields that got reset by the REST endpoint +func (r *StatusREST) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set { + return r.store.GetResetFields() +} + +// ConvertToTable delegates to the store, implements TableConverter +func (r *StatusREST) ConvertToTable(ctx context.Context, object runtime.Object, tableOptions runtime.Object) (*metav1.Table, error) { + return r.store.ConvertToTable(ctx, object, tableOptions) +} + +// Update alters the status subset of an object. Delegates to the store +func (r *StatusREST) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) { + return r.store.Update(ctx, name, objInfo, createValidation, updateValidation, forceAllowCreate, options) +} diff --git a/pkg/registry/admissionregistration/validatingadmissionpolicy/storage/storage_test.go b/pkg/registry/admissionregistration/validatingadmissionpolicy/storage/storage_test.go index 02c2d134848..29c35559873 100644 --- a/pkg/registry/admissionregistration/validatingadmissionpolicy/storage/storage_test.go +++ b/pkg/registry/admissionregistration/validatingadmissionpolicy/storage/storage_test.go @@ -211,7 +211,7 @@ func newStorage(t *testing.T, authorizer authorizer.Authorizer, resourceResolver Decorator: generic.UndecoratedStorage, DeleteCollectionWorkers: 1, ResourcePrefix: "validatingadmissionpolicies"} - storage, err := NewREST(restOptions, authorizer, resourceResolver) + storage, _, err := NewREST(restOptions, authorizer, resourceResolver) if err != nil { t.Fatalf("unexpected error from REST storage: %v", err) } diff --git a/pkg/registry/admissionregistration/validatingadmissionpolicy/strategy.go b/pkg/registry/admissionregistration/validatingadmissionpolicy/strategy.go index 70bc547048f..2f980c4c82c 100644 --- a/pkg/registry/admissionregistration/validatingadmissionpolicy/strategy.go +++ b/pkg/registry/admissionregistration/validatingadmissionpolicy/strategy.go @@ -19,7 +19,10 @@ package validatingadmissionpolicy import ( "context" + "sigs.k8s.io/structured-merge-diff/v4/fieldpath" + apiequality "k8s.io/apimachinery/pkg/api/equality" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/authorization/authorizer" @@ -56,6 +59,7 @@ func (v *validatingAdmissionPolicyStrategy) NamespaceScoped() bool { // PrepareForCreate clears the status of an validatingAdmissionPolicy before creation. func (v *validatingAdmissionPolicyStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { ic := obj.(*admissionregistration.ValidatingAdmissionPolicy) + ic.Status = admissionregistration.ValidatingAdmissionPolicyStatus{} ic.Generation = 1 } @@ -64,6 +68,9 @@ func (v *validatingAdmissionPolicyStrategy) PrepareForUpdate(ctx context.Context newIC := obj.(*admissionregistration.ValidatingAdmissionPolicy) oldIC := old.(*admissionregistration.ValidatingAdmissionPolicy) + // Prevent any update on the Status object + newIC.Status = oldIC.Status + // Any changes to the spec increment the generation number, any changes to the // status should reflect the generation number of the corresponding object. // See metav1.ObjectMeta description for more information on Generation. @@ -120,3 +127,53 @@ func (v *validatingAdmissionPolicyStrategy) WarningsOnUpdate(ctx context.Context func (v *validatingAdmissionPolicyStrategy) AllowUnconditionalUpdate() bool { return false } + +// GetResetFields returns the set of fields that get reset by the strategy +// and should not be modified by the user. +func (v *validatingAdmissionPolicyStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set { + fields := map[fieldpath.APIVersion]*fieldpath.Set{ + "admissionregistration.k8s.io/v1alpha1": fieldpath.NewSet( + fieldpath.MakePathOrDie("status"), + ), + } + + return fields +} + +type validatingAdmissionPolicyStatusStrategy struct { + *validatingAdmissionPolicyStrategy +} + +// ValidateUpdate is the default update validation of an update to the status object. +func (s *validatingAdmissionPolicyStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { + return validation.ValidateValidatingAdmissionPolicyStatusUpdate(obj.(*admissionregistration.ValidatingAdmissionPolicy), old.(*admissionregistration.ValidatingAdmissionPolicy)) +} + +// PrepareForUpdate differs from the main strategy where setting the spec is not +// allowed, but setting status is OK. +func (s *validatingAdmissionPolicyStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { + newIC := obj.(*admissionregistration.ValidatingAdmissionPolicy) + oldIC := old.(*admissionregistration.ValidatingAdmissionPolicy) + + // Prevent any update on the Spec object from Status Strategy + newIC.Spec = oldIC.Spec + + metav1.ResetObjectMetaForStatus(&newIC.ObjectMeta, &oldIC.ObjectMeta) + // No change in the generation. +} + +// GetResetFields returns the set of fields that get reset by the strategy +// and should not be modified by the user. +func (s *validatingAdmissionPolicyStatusStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set { + return map[fieldpath.APIVersion]*fieldpath.Set{ + "admissionregistration.k8s.io/v1alpha1": fieldpath.NewSet( + fieldpath.MakePathOrDie("spec"), + fieldpath.MakePathOrDie("metadata"), + ), + } +} + +// NewStatusStrategy creates a strategy for operating the status object. +func NewStatusStrategy(policyStrategy *validatingAdmissionPolicyStrategy) *validatingAdmissionPolicyStatusStrategy { + return &validatingAdmissionPolicyStatusStrategy{validatingAdmissionPolicyStrategy: policyStrategy} +} diff --git a/staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go b/staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go index 299751c0210..ec7a1301783 100644 --- a/staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go +++ b/staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go @@ -74,6 +74,49 @@ type ValidatingAdmissionPolicy struct { metav1.ObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` // Specification of the desired behavior of the ValidatingAdmissionPolicy. Spec ValidatingAdmissionPolicySpec `json:"spec,omitempty" protobuf:"bytes,2,opt,name=spec"` + // The status of the ValidatingAdmissionPolicy, including warnings that are useful to determine if the policy + // behaves in the expected way. + // Populated by the system. + // Read-only. + // +optional + Status ValidatingAdmissionPolicyStatus `json:"status,omitempty" protobuf:"bytes,3,opt,name=status"` +} + +// ValidatingAdmissionPolicyStatus represents the status of a ValidatingAdmissionPolicy. +type ValidatingAdmissionPolicyStatus struct { + // The generation observed by the controller. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty" protobuf:"varint,1,opt,name=observedGeneration"` + // The results of type checking for each expression. + // Presence of this field indicates the completion of the type checking. + // +optional + TypeChecking *TypeChecking `json:"typeChecking,omitempty" protobuf:"bytes,2,opt,name=typeChecking"` + // The conditions represent the latest available observations of a policy's current state. + // +optional + // +listType=map + // +listMapKey=type + Conditions []metav1.Condition `json:"conditions,omitempty" protobuf:"bytes,3,rep,name=conditions"` +} + +// TypeChecking contains results of type checking the expressions in the +// ValidatingAdmissionPolicy +type TypeChecking struct { + // The type checking warnings for each expression. + // +optional + // +listType=atomic + ExpressionWarnings []ExpressionWarning `json:"expressionWarnings,omitempty" protobuf:"bytes,1,rep,name=expressionWarnings"` +} + +// ExpressionWarning is a warning information that targets a specific expression. +type ExpressionWarning struct { + // The path to the field that refers the expression. + // For example, the reference to the expression of the first item of + // validations is "spec.validations[0].expression" + FieldRef string `json:"fieldRef" protobuf:"bytes,2,opt,name=fieldRef"` + // The content of type checking information in a human-readable form. + // Each line of the warning contains the type that the expression is checked + // against, followed by the type check error from the compiler. + Warning string `json:"warning" protobuf:"bytes,3,opt,name=warning"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object