From bff8a6cd9f38be170a939cfd86b0c80f61504392 Mon Sep 17 00:00:00 2001 From: Ricardo Katz Date: Thu, 16 Feb 2023 13:51:04 -0300 Subject: [PATCH] Remove withdrawn feature NetworkPolicyStatus --- pkg/apis/networking/types.go | 41 ---- pkg/apis/networking/validation/validation.go | 5 - .../networking/validation/validation_test.go | 133 ------------- pkg/features/kube_features.go | 9 - .../networkpolicy/storage/storage.go | 55 +----- .../networkpolicy/storage/storage_test.go | 102 +--------- .../networking/networkpolicy/strategy.go | 84 --------- .../networking/networkpolicy/strategy_test.go | 176 ------------------ .../networking/rest/storage_settings.go | 3 +- .../k8s.io/api/extensions/v1beta1/types.go | 50 +---- .../api/extensions/v1beta1/types_test.go | 42 +++++ staging/src/k8s.io/api/networking/v1/types.go | 50 +---- .../k8s.io/api/networking/v1/types_test.go | 42 +++++ .../testdata/ineligible_endpoints.yaml | 9 +- test/e2e/network/netpol/network_policy_api.go | 75 -------- .../apiserver/apply/reset_fields_test.go | 9 - .../apiserver/apply/status_test.go | 2 - 17 files changed, 115 insertions(+), 772 deletions(-) create mode 100644 staging/src/k8s.io/api/extensions/v1beta1/types_test.go create mode 100644 staging/src/k8s.io/api/networking/v1/types_test.go diff --git a/pkg/apis/networking/types.go b/pkg/apis/networking/types.go index 6c9eaa7a484..9ec17540bae 100644 --- a/pkg/apis/networking/types.go +++ b/pkg/apis/networking/types.go @@ -35,11 +35,6 @@ type NetworkPolicy struct { // spec represents the specification of the desired behavior for this NetworkPolicy. // +optional Spec NetworkPolicySpec - - // status represents the current state of the NetworkPolicy. - // More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status - // +optional - Status NetworkPolicyStatus } // PolicyType describes the NetworkPolicy type @@ -201,42 +196,6 @@ type NetworkPolicyPeer struct { IPBlock *IPBlock } -// NetworkPolicyConditionType is the type for status conditions on -// a NetworkPolicy. This type should be used with the -// NetworkPolicyStatus.Conditions field. -type NetworkPolicyConditionType string - -const ( - // NetworkPolicyConditionStatusAccepted represents status of a Network Policy that could be properly parsed by - // the Network Policy provider and will be implemented in the cluster - NetworkPolicyConditionStatusAccepted NetworkPolicyConditionType = "Accepted" - - // NetworkPolicyConditionStatusPartialFailure represents status of a Network Policy that could be partially - // parsed by the Network Policy provider and may not be completely implemented due to a lack of a feature or some - // other condition - NetworkPolicyConditionStatusPartialFailure NetworkPolicyConditionType = "PartialFailure" - - // NetworkPolicyConditionStatusFailure represents status of a Network Policy that could not be parsed by the - // Network Policy provider and will not be implemented in the cluster - NetworkPolicyConditionStatusFailure NetworkPolicyConditionType = "Failure" -) - -// NetworkPolicyConditionReason defines the set of reasons that explain why a -// particular NetworkPolicy condition type has been raised. -type NetworkPolicyConditionReason string - -const ( - // NetworkPolicyConditionReasonFeatureNotSupported represents a reason where the Network Policy may not have been - // implemented in the cluster due to a lack of some feature not supported by the Network Policy provider - NetworkPolicyConditionReasonFeatureNotSupported NetworkPolicyConditionReason = "FeatureNotSupported" -) - -// NetworkPolicyStatus describes the current state of the NetworkPolicy. -type NetworkPolicyStatus struct { - // conditions holds an array of metav1.Condition that describes the state of the NetworkPolicy. - Conditions []metav1.Condition -} - // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // NetworkPolicyList is a list of NetworkPolicy objects. diff --git a/pkg/apis/networking/validation/validation.go b/pkg/apis/networking/validation/validation.go index d0ec25726c4..84928914718 100644 --- a/pkg/apis/networking/validation/validation.go +++ b/pkg/apis/networking/validation/validation.go @@ -214,11 +214,6 @@ func ValidateNetworkPolicyUpdate(update, old *networking.NetworkPolicy, opts Net return allErrs } -// ValidateNetworkPolicyStatusUpdate tests if an update to a NetworkPolicy status is valid -func ValidateNetworkPolicyStatusUpdate(status, oldstatus networking.NetworkPolicyStatus, fldPath *field.Path) field.ErrorList { - return unversionedvalidation.ValidateConditions(status.Conditions, fldPath.Child("conditions")) -} - // ValidateIPBlock validates a cidr and the except fields of an IpBlock NetworkPolicyPeer func ValidateIPBlock(ipb *networking.IPBlock, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} diff --git a/pkg/apis/networking/validation/validation_test.go b/pkg/apis/networking/validation/validation_test.go index 7884ddeb370..4a5d71d9ac1 100644 --- a/pkg/apis/networking/validation/validation_test.go +++ b/pkg/apis/networking/validation/validation_test.go @@ -20,7 +20,6 @@ import ( "fmt" "strings" "testing" - "time" apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -456,138 +455,6 @@ func TestValidateNetworkPolicyUpdate(t *testing.T) { } } -func TestValidateNetworkPolicyStatusUpdate(t *testing.T) { - - type netpolStatusCases struct { - obj networking.NetworkPolicyStatus - expectedErrs field.ErrorList - } - - testCases := map[string]netpolStatusCases{ - "valid conditions": { - obj: networking.NetworkPolicyStatus{ - Conditions: []metav1.Condition{ - { - Type: string(networking.NetworkPolicyConditionStatusAccepted), - Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Time{ - Time: time.Now().Add(-5 * time.Minute), - }, - Reason: "RuleApplied", - Message: "rule was successfully applied", - ObservedGeneration: 2, - }, - { - Type: string(networking.NetworkPolicyConditionStatusFailure), - Status: metav1.ConditionFalse, - LastTransitionTime: metav1.Time{ - Time: time.Now().Add(-5 * time.Minute), - }, - Reason: "RuleApplied", - Message: "no error was found", - ObservedGeneration: 2, - }, - }, - }, - expectedErrs: field.ErrorList{}, - }, - "duplicate type": { - obj: networking.NetworkPolicyStatus{ - Conditions: []metav1.Condition{ - { - Type: string(networking.NetworkPolicyConditionStatusAccepted), - Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Time{ - Time: time.Now().Add(-5 * time.Minute), - }, - Reason: "RuleApplied", - Message: "rule was successfully applied", - ObservedGeneration: 2, - }, - { - Type: string(networking.NetworkPolicyConditionStatusAccepted), - Status: metav1.ConditionFalse, - LastTransitionTime: metav1.Time{ - Time: time.Now().Add(-5 * time.Minute), - }, - Reason: string(networking.NetworkPolicyConditionReasonFeatureNotSupported), - Message: "endport is not supported", - ObservedGeneration: 2, - }, - }, - }, - expectedErrs: field.ErrorList{field.Duplicate(field.NewPath("status").Child("conditions").Index(1).Child("type"), - string(networking.NetworkPolicyConditionStatusAccepted))}, - }, - "invalid generation": { - obj: networking.NetworkPolicyStatus{ - Conditions: []metav1.Condition{ - { - Type: string(networking.NetworkPolicyConditionStatusAccepted), - Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Time{ - Time: time.Now().Add(-5 * time.Minute), - }, - Reason: "RuleApplied", - Message: "rule was successfully applied", - ObservedGeneration: -1, - }, - }, - }, - expectedErrs: field.ErrorList{field.Invalid(field.NewPath("status").Child("conditions").Index(0).Child("observedGeneration"), - int64(-1), "must be greater than or equal to zero")}, - }, - "invalid null transition time": { - obj: networking.NetworkPolicyStatus{ - Conditions: []metav1.Condition{ - { - Type: string(networking.NetworkPolicyConditionStatusAccepted), - Status: metav1.ConditionTrue, - Reason: "RuleApplied", - Message: "rule was successfully applied", - ObservedGeneration: 3, - }, - }, - }, - expectedErrs: field.ErrorList{field.Required(field.NewPath("status").Child("conditions").Index(0).Child("lastTransitionTime"), - "must be set")}, - }, - "multiple condition errors": { - obj: networking.NetworkPolicyStatus{ - Conditions: []metav1.Condition{ - { - Type: string(networking.NetworkPolicyConditionStatusAccepted), - Status: metav1.ConditionTrue, - Reason: "RuleApplied", - Message: "rule was successfully applied", - ObservedGeneration: -1, - }, - }, - }, - expectedErrs: field.ErrorList{ - field.Invalid(field.NewPath("status").Child("conditions").Index(0).Child("observedGeneration"), - int64(-1), "must be greater than or equal to zero"), - field.Required(field.NewPath("status").Child("conditions").Index(0).Child("lastTransitionTime"), - "must be set"), - }, - }, - } - - for testName, testCase := range testCases { - errs := ValidateNetworkPolicyStatusUpdate(testCase.obj, networking.NetworkPolicyStatus{}, field.NewPath("status")) - if len(errs) != len(testCase.expectedErrs) { - t.Errorf("Test %s: Expected %d errors, got %d (%+v)", testName, len(testCase.expectedErrs), len(errs), errs) - } - - for i, err := range errs { - if err.Error() != testCase.expectedErrs[i].Error() { - t.Errorf("Test %s: Expected error: %v, got %v", testName, testCase.expectedErrs[i], err) - } - } - } - -} - func TestValidateIngress(t *testing.T) { serviceBackend := &networking.IngressServiceBackend{ Name: "defaultbackend", diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 4bba72caea9..e90437160fb 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -560,13 +560,6 @@ const ( // Enables the dynamic configuration of Service IP ranges MultiCIDRServiceAllocator featuregate.Feature = "MultiCIDRServiceAllocator" - // owner: @rikatz - // kep: https://kep.k8s.io/2943 - // alpha: v1.24 - // - // Enables NetworkPolicy status subresource - NetworkPolicyStatus featuregate.Feature = "NetworkPolicyStatus" - // owner: @jsafrane // kep: https://kep.k8s.io/3756 // alpha: v1.25 (as part of SELinuxMountReadWriteOncePod) @@ -1025,8 +1018,6 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS MultiCIDRServiceAllocator: {Default: false, PreRelease: featuregate.Alpha}, - NetworkPolicyStatus: {Default: false, PreRelease: featuregate.Alpha}, - NewVolumeManagerReconstruction: {Default: true, PreRelease: featuregate.Beta}, NodeLogQuery: {Default: false, PreRelease: featuregate.Alpha}, diff --git a/pkg/registry/networking/networkpolicy/storage/storage.go b/pkg/registry/networking/networkpolicy/storage/storage.go index 0445c98a4c2..9e91913d7ab 100644 --- a/pkg/registry/networking/networkpolicy/storage/storage.go +++ b/pkg/registry/networking/networkpolicy/storage/storage.go @@ -17,11 +17,6 @@ limitations under the License. package storage import ( - "context" - - "sigs.k8s.io/structured-merge-diff/v4/fieldpath" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/generic" genericregistry "k8s.io/apiserver/pkg/registry/generic/registry" @@ -39,30 +34,25 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against NetworkPolicies. -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &networkingapi.NetworkPolicy{} }, NewListFunc: func() runtime.Object { return &networkingapi.NetworkPolicyList{} }, DefaultQualifiedResource: networkingapi.Resource("networkpolicies"), SingularQualifiedResource: networkingapi.Resource("networkpolicy"), - CreateStrategy: networkpolicy.Strategy, - UpdateStrategy: networkpolicy.Strategy, - DeleteStrategy: networkpolicy.Strategy, - ResetFieldsStrategy: networkpolicy.Strategy, + CreateStrategy: networkpolicy.Strategy, + UpdateStrategy: networkpolicy.Strategy, + DeleteStrategy: networkpolicy.Strategy, TableConvertor: printerstorage.TableConvertor{TableGenerator: printers.NewTableGenerator().With(printersinternal.AddHandlers)}, } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - return nil, nil, err + return nil, err } - statusStore := *store - statusStore.UpdateStrategy = networkpolicy.StatusStrategy - statusStore.ResetFieldsStrategy = networkpolicy.StatusStrategy - return &REST{store}, &StatusREST{store: &statusStore}, nil - + return &REST{store}, nil } // Implement ShortNamesProvider @@ -72,36 +62,3 @@ var _ rest.ShortNamesProvider = &REST{} func (r *REST) ShortNames() []string { return []string{"netpol"} } - -// StatusREST implements the REST endpoint for changing the status of an ingress -type StatusREST struct { - store *genericregistry.Store -} - -// New creates an instance of the StatusREST object -func (r *StatusREST) New() runtime.Object { - return &networkingapi.NetworkPolicy{} -} - -// Destroy cleans up resources on shutdown. -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) -} - -// Update alters the status subset of an object. -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) { - // We are explicitly setting forceAllowCreate to false in the call to the underlying storage because - // subresources should never allow create on update. - return r.store.Update(ctx, name, objInfo, createValidation, updateValidation, false, options) -} - -// GetResetFields implements rest.ResetFieldsStrategy -func (r *StatusREST) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set { - return r.store.GetResetFields() -} diff --git a/pkg/registry/networking/networkpolicy/storage/storage_test.go b/pkg/registry/networking/networkpolicy/storage/storage_test.go index 15ab8780910..611f8db8905 100644 --- a/pkg/registry/networking/networkpolicy/storage/storage_test.go +++ b/pkg/registry/networking/networkpolicy/storage/storage_test.go @@ -18,28 +18,22 @@ package storage import ( "testing" - "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" - genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/generic" genericregistrytest "k8s.io/apiserver/pkg/registry/generic/testing" - "k8s.io/apiserver/pkg/registry/rest" etcd3testing "k8s.io/apiserver/pkg/storage/etcd3/testing" - utilfeature "k8s.io/apiserver/pkg/util/feature" - featuregatetesting "k8s.io/component-base/featuregate/testing" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/networking" _ "k8s.io/kubernetes/pkg/apis/networking/install" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/registry/registrytest" ) -func newStorage(t *testing.T) (*REST, *StatusREST, *etcd3testing.EtcdTestServer) { +func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { etcdStorage, server := registrytest.NewEtcdStorage(t, networking.GroupName) restOptions := generic.RESTOptions{ StorageConfig: etcdStorage, @@ -47,11 +41,11 @@ func newStorage(t *testing.T) (*REST, *StatusREST, *etcd3testing.EtcdTestServer) DeleteCollectionWorkers: 1, ResourcePrefix: "networkpolicies", } - rest, status, err := NewREST(restOptions) + rest, err := NewREST(restOptions) if err != nil { t.Fatalf("unexpected error from REST storage: %v", err) } - return rest, status, server + return rest, server } func validNetworkPolicy() *networking.NetworkPolicy { @@ -76,12 +70,11 @@ func validNetworkPolicy() *networking.NetworkPolicy { }, }, }, - Status: networking.NetworkPolicyStatus{}, } } func TestCreate(t *testing.T) { - storage, _, server := newStorage(t) + storage, server := newStorage(t) defer server.Terminate(t) defer storage.Store.DestroyFunc() test := genericregistrytest.New(t, storage.Store) @@ -99,7 +92,7 @@ func TestCreate(t *testing.T) { func TestUpdate(t *testing.T) { protocolICMP := api.Protocol("ICMP") - storage, _, server := newStorage(t) + storage, server := newStorage(t) defer server.Terminate(t) defer storage.Store.DestroyFunc() test := genericregistrytest.New(t, storage.Store) @@ -142,7 +135,7 @@ func TestUpdate(t *testing.T) { } func TestDelete(t *testing.T) { - storage, _, server := newStorage(t) + storage, server := newStorage(t) defer server.Terminate(t) defer storage.Store.DestroyFunc() test := genericregistrytest.New(t, storage.Store) @@ -150,7 +143,7 @@ func TestDelete(t *testing.T) { } func TestGet(t *testing.T) { - storage, _, server := newStorage(t) + storage, server := newStorage(t) defer server.Terminate(t) defer storage.Store.DestroyFunc() test := genericregistrytest.New(t, storage.Store) @@ -158,7 +151,7 @@ func TestGet(t *testing.T) { } func TestList(t *testing.T) { - storage, _, server := newStorage(t) + storage, server := newStorage(t) defer server.Terminate(t) defer storage.Store.DestroyFunc() test := genericregistrytest.New(t, storage.Store) @@ -166,7 +159,7 @@ func TestList(t *testing.T) { } func TestWatch(t *testing.T) { - storage, _, server := newStorage(t) + storage, server := newStorage(t) defer server.Terminate(t) defer storage.Store.DestroyFunc() test := genericregistrytest.New(t, storage.Store) @@ -191,84 +184,9 @@ func TestWatch(t *testing.T) { } func TestShortNames(t *testing.T) { - storage, _, server := newStorage(t) + storage, server := newStorage(t) defer server.Terminate(t) defer storage.Store.DestroyFunc() expected := []string{"netpol"} registrytest.AssertShortNames(t, storage, expected) } - -func TestStatusUpdate(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.NetworkPolicyStatus, true)() - storage, statusStorage, server := newStorage(t) - defer server.Terminate(t) - defer storage.Store.DestroyFunc() - ctx := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceDefault) - key := "/networkpolicies/" + metav1.NamespaceDefault + "/foo" - validNetPolObject := validNetworkPolicy() - if err := storage.Storage.Create(ctx, key, validNetPolObject, nil, 0, false); err != nil { - t.Fatalf("unexpected error: %v", err) - } - - obj, err := storage.Get(ctx, "foo", &metav1.GetOptions{}) - if err != nil { - t.Fatalf("failed to get netpol: %v", err) - } - - obtainedNetPol := obj.(*networking.NetworkPolicy) - - transition := time.Now().Add(-5 * time.Minute) - update := networking.NetworkPolicy{ - ObjectMeta: obtainedNetPol.ObjectMeta, - Spec: obtainedNetPol.Spec, - Status: networking.NetworkPolicyStatus{ - Conditions: []metav1.Condition{ - { - Type: string(networking.NetworkPolicyConditionStatusAccepted), - Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Time{ - Time: transition, - }, - Reason: "RuleApplied", - Message: "rule was successfully applied", - ObservedGeneration: 2, - }, - }, - }, - } - - if _, _, err := statusStorage.Update(ctx, update.Name, rest.DefaultUpdatedObjectInfo(&update), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{}); err != nil { - t.Fatalf("unexpected error: %v", err) - } - obj, err = storage.Get(ctx, "foo", &metav1.GetOptions{}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - netpol := obj.(*networking.NetworkPolicy) - if len(netpol.Status.Conditions) != 1 { - t.Fatalf("we expected 1 condition to exist in status but %d occurred", len(netpol.Status.Conditions)) - } - - condition := netpol.Status.Conditions[0] - if condition.Type != string(networking.NetworkPolicyConditionStatusAccepted) { - t.Errorf("we expected condition type to be %s but %s was returned", string(networking.NetworkPolicyConditionStatusAccepted), condition.Type) - } - - if condition.Status != metav1.ConditionTrue { - t.Errorf("we expected condition status to be true, but it returned false") - } - - if condition.Reason != "RuleApplied" { - t.Errorf("we expected condition reason to be RuleApplied, but %s was returned", condition.Reason) - } - - if condition.Message != "rule was successfully applied" { - t.Errorf("we expected message to be 'rule was successfully applied', but %s was returned", condition.Message) - } - - if condition.ObservedGeneration != 2 { - t.Errorf("we expected observedGeneration to be 2, but %d was returned", condition.ObservedGeneration) - } - -} diff --git a/pkg/registry/networking/networkpolicy/strategy.go b/pkg/registry/networking/networkpolicy/strategy.go index 692ce54168d..4e93da62a77 100644 --- a/pkg/registry/networking/networkpolicy/strategy.go +++ b/pkg/registry/networking/networkpolicy/strategy.go @@ -20,16 +20,12 @@ import ( "context" "reflect" - "sigs.k8s.io/structured-merge-diff/v4/fieldpath" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/storage/names" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/apis/networking" "k8s.io/kubernetes/pkg/apis/networking/validation" - "k8s.io/kubernetes/pkg/features" ) // networkPolicyStrategy implements verification logic for NetworkPolicies @@ -46,31 +42,10 @@ func (networkPolicyStrategy) NamespaceScoped() bool { return true } -// GetResetFields returns the set of fields that get reset by the strategy -// and should not be modified by the user. -func (networkPolicyStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set { - fields := map[fieldpath.APIVersion]*fieldpath.Set{ - "extensions/v1beta1": fieldpath.NewSet( - fieldpath.MakePathOrDie("status"), - ), - "networking.k8s.io/v1": fieldpath.NewSet( - fieldpath.MakePathOrDie("status"), - ), - } - return fields -} - // PrepareForCreate clears the status of a NetworkPolicy before creation. func (networkPolicyStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { networkPolicy := obj.(*networking.NetworkPolicy) - - if utilfeature.DefaultFeatureGate.Enabled(features.NetworkPolicyStatus) { - // Create does not set a status when operation is not directed to status subresource - networkPolicy.Status = networking.NetworkPolicyStatus{} - } - networkPolicy.Generation = 1 - } // PrepareForUpdate clears fields that are not allowed to be set by end users on update. @@ -78,13 +53,6 @@ func (networkPolicyStrategy) PrepareForUpdate(ctx context.Context, obj, old runt newNetworkPolicy := obj.(*networking.NetworkPolicy) oldNetworkPolicy := old.(*networking.NetworkPolicy) - // We copy the status if the FG is enabled, or if previously there was already data on the conditions field - // As soon as the FeatureGate is removed, the whole if statement should be removed as well - if utilfeature.DefaultFeatureGate.Enabled(features.NetworkPolicyStatus) || len(oldNetworkPolicy.Status.Conditions) > 0 { - // Update is not allowed to set status when the operation is not directed to status subresource - newNetworkPolicy.Status = oldNetworkPolicy.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. @@ -130,55 +98,3 @@ func (networkPolicyStrategy) WarningsOnUpdate(ctx context.Context, obj, old runt func (networkPolicyStrategy) AllowUnconditionalUpdate() bool { return true } - -type networkPolicyStatusStrategy struct { - networkPolicyStrategy -} - -// StatusStrategy implements logic used to validate and prepare for updates of the status subresource -var StatusStrategy = networkPolicyStatusStrategy{Strategy} - -// GetResetFields returns the set of fields that get reset by the strategy -// and should not be modified by the user. -func (networkPolicyStatusStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set { - fields := map[fieldpath.APIVersion]*fieldpath.Set{ - "extensions/v1beta1": fieldpath.NewSet( - fieldpath.MakePathOrDie("spec"), - ), - "networking.k8s.io/v1": fieldpath.NewSet( - fieldpath.MakePathOrDie("spec"), - ), - } - return fields -} - -// PrepareForUpdate clears fields that are not allowed to be set by end users on update of status -func (networkPolicyStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { - newNetworkPolicy := obj.(*networking.NetworkPolicy) - oldNetworkPolicy := old.(*networking.NetworkPolicy) - // status changes are not allowed to update spec - newNetworkPolicy.Spec = oldNetworkPolicy.Spec - - if !utilfeature.DefaultFeatureGate.Enabled(features.NetworkPolicyStatus) { - // As network policy status is composed only of an array of conditions, we can say that the status - // is in use if the condition array is bigger than 0. - // quoting @thockin: "we generally keep data in this case, but no updates except to clear it" - if len(newNetworkPolicy.Status.Conditions) == 0 { - newNetworkPolicy.Status = networking.NetworkPolicyStatus{} - } else { - // keep the old status in case of the update is not to clear it - newNetworkPolicy.Status = oldNetworkPolicy.Status - } - } -} - -// ValidateUpdate is the default update validation for an end user updating status -func (networkPolicyStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - return validation.ValidateNetworkPolicyStatusUpdate(obj.(*networking.NetworkPolicy).Status, - old.(*networking.NetworkPolicy).Status, field.NewPath("status")) -} - -// WarningsOnUpdate returns warnings for the given update. -func (networkPolicyStatusStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string { - return nil -} diff --git a/pkg/registry/networking/networkpolicy/strategy_test.go b/pkg/registry/networking/networkpolicy/strategy_test.go index 1f03838acc4..b5eca01299c 100644 --- a/pkg/registry/networking/networkpolicy/strategy_test.go +++ b/pkg/registry/networking/networkpolicy/strategy_test.go @@ -18,20 +18,12 @@ package networkpolicy import ( "context" - "reflect" "testing" - "time" - - genericapirequest "k8s.io/apiserver/pkg/endpoints/request" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/networking" - "k8s.io/kubernetes/pkg/features" - - utilfeature "k8s.io/apiserver/pkg/util/feature" - featuregatetesting "k8s.io/component-base/featuregate/testing" ) func makeNetworkPolicy(isIngress, isEgress, hasEndPort bool) *networking.NetworkPolicy { @@ -119,171 +111,3 @@ func TestNetworkPolicyStrategy(t *testing.T) { } } - -func TestNetworkPolicyStatusStrategy(t *testing.T) { - for _, tc := range []struct { - name string - enableFeatureGate bool - invalidStatus bool - }{ - { - name: "Update NetworkPolicy status with FeatureGate enabled", - enableFeatureGate: true, - invalidStatus: false, - }, - { - name: "Update NetworkPolicy status with FeatureGate disabled", - enableFeatureGate: false, - invalidStatus: false, - }, - { - name: "Update NetworkPolicy status with FeatureGate enabled and invalid status", - enableFeatureGate: true, - invalidStatus: true, - }, - { - name: "Update NetworkPolicy status with FeatureGate disabled and invalid status", - enableFeatureGate: false, - invalidStatus: true, - }, - } { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.NetworkPolicyStatus, tc.enableFeatureGate)() - ctx := genericapirequest.NewDefaultContext() - if !StatusStrategy.NamespaceScoped() { - t.Errorf("NetworkPolicy must be namespace scoped") - } - if StatusStrategy.AllowCreateOnUpdate() { - t.Errorf("NetworkPolicy should not allow create on update") - } - - oldNetPol := makeNetworkPolicy(false, true, false) - newNetPol := makeNetworkPolicy(true, true, true) - newNetPol.Status = networking.NetworkPolicyStatus{ - Conditions: []metav1.Condition{ - { - Type: string(networking.NetworkPolicyConditionStatusAccepted), - Status: metav1.ConditionTrue, - Reason: "RuleApplied", - Message: "rule was successfully applied", - ObservedGeneration: 2, - }, - }, - } - if !tc.invalidStatus { - newNetPol.Status.Conditions[0].LastTransitionTime = metav1.Time{Time: time.Now().Add(-5 * time.Minute)} - } - - StatusStrategy.PrepareForUpdate(ctx, newNetPol, oldNetPol) - if tc.enableFeatureGate { - if !reflect.DeepEqual(oldNetPol.Spec, newNetPol.Spec) { - t.Errorf("status update should not change network policy spec") - } - if len(newNetPol.Status.Conditions) != 1 { - t.Fatalf("expecting 1 condition in network policy, got %d", len(newNetPol.Status.Conditions)) - } - - if newNetPol.Status.Conditions[0].Type != string(networking.NetworkPolicyConditionStatusAccepted) { - t.Errorf("NetworkPolicy status updates should allow change of condition fields") - } - } else { - if len(newNetPol.Status.Conditions) != 0 && !tc.enableFeatureGate { - t.Fatalf("expecting 0 condition in network policy, got %d", len(newNetPol.Status.Conditions)) - } - } - - errs := StatusStrategy.ValidateUpdate(ctx, newNetPol, oldNetPol) - if tc.enableFeatureGate { - if tc.invalidStatus && len(errs) == 0 { - t.Error("invalid network policy status wasn't proper validated") - } - if !tc.invalidStatus && len(errs) > 0 { - t.Errorf("valid network policy status returned an error: %v", errs) - } - } else { - if len(errs) != 0 { - t.Errorf("Unexpected error with disabled featuregate: %v", errs) - } - } - - } -} - -// This test will verify the behavior of NetworkPolicy Status when enabling/disabling/re-enabling the feature gate -func TestNetworkPolicyStatusStrategyEnablement(t *testing.T) { - // Enable the Feature Gate during the first rule creation - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.NetworkPolicyStatus, true)() - ctx := genericapirequest.NewDefaultContext() - - oldNetPol := makeNetworkPolicy(false, true, false) - newNetPol := makeNetworkPolicy(true, true, false) - newNetPol.Status = networking.NetworkPolicyStatus{ - Conditions: []metav1.Condition{ - { - Type: string(networking.NetworkPolicyConditionStatusAccepted), - Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Time{Time: time.Now().Add(-5 * time.Minute)}, - Reason: "RuleApplied", - Message: "rule was successfully applied", - ObservedGeneration: 2, - }, - }, - } - - StatusStrategy.PrepareForUpdate(ctx, newNetPol, oldNetPol) - - if !reflect.DeepEqual(oldNetPol.Spec, newNetPol.Spec) { - t.Errorf("status update should not change network policy spec") - } - - if len(newNetPol.Status.Conditions) != 1 || newNetPol.Status.Conditions[0].Status != metav1.ConditionTrue { - t.Error("expected network policy status is incorrect") - } - - // Now let's disable the Feature Gate, update some other field from NetPol and expect the Status is already present - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.NetworkPolicyStatus, false)() - - oldNetPol = newNetPol.DeepCopy() - // 1 - It should not allow to change status, and just copy between objects when FG is disabled - newNetPol.Status.Conditions[0].Status = metav1.ConditionFalse - - StatusStrategy.PrepareForUpdate(ctx, newNetPol, oldNetPol) - if len(newNetPol.Status.Conditions) != 1 { - t.Fatalf("expected conditions after disabling feature is invalid: got %d and expected 1", len(newNetPol.Status.Conditions)) - } - - if newNetPol.Status.Conditions[0].Status != metav1.ConditionTrue { - t.Error("condition status changed with feature gate disabled") - } - - oldNetPol = newNetPol.DeepCopy() - // 2 - It should clear status if it contained previous data and is disabled now - newNetPol.Status = networking.NetworkPolicyStatus{} - StatusStrategy.PrepareForUpdate(ctx, newNetPol, oldNetPol) - if len(newNetPol.Status.Conditions) != 0 { - t.Errorf("expected conditions after disabling feature and cleaning status is invalid: got %d and expected 0", len(newNetPol.Status.Conditions)) - } - - oldNetPol = newNetPol.DeepCopy() - // 3 - It should allow again to add status when re-enabling the FG - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.NetworkPolicyStatus, true)() - - newNetPol.Status = networking.NetworkPolicyStatus{ - Conditions: []metav1.Condition{ - { - Type: string(networking.NetworkPolicyConditionStatusAccepted), - Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Time{Time: time.Now().Add(-5 * time.Minute)}, - Reason: "RuleApplied", - Message: "rule was successfully applied", - ObservedGeneration: 2, - }, - }, - } - - StatusStrategy.PrepareForUpdate(ctx, newNetPol, oldNetPol) - - if len(newNetPol.Status.Conditions) != 1 || newNetPol.Status.Conditions[0].Status != metav1.ConditionTrue { - t.Error("expected network policy status is incorrect") - } - -} diff --git a/pkg/registry/networking/rest/storage_settings.go b/pkg/registry/networking/rest/storage_settings.go index c81f0deb85a..1ca958f9c8c 100644 --- a/pkg/registry/networking/rest/storage_settings.go +++ b/pkg/registry/networking/rest/storage_settings.go @@ -59,12 +59,11 @@ func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.API // networkpolicies if resource := "networkpolicies"; apiResourceConfigSource.ResourceEnabled(networkingapiv1.SchemeGroupVersion.WithResource(resource)) { - networkPolicyStorage, networkPolicyStatusStorage, err := networkpolicystore.NewREST(restOptionsGetter) + networkPolicyStorage, err := networkpolicystore.NewREST(restOptionsGetter) if err != nil { return storage, err } storage[resource] = networkPolicyStorage - storage[resource+"/status"] = networkPolicyStatusStorage } // ingresses diff --git a/staging/src/k8s.io/api/extensions/v1beta1/types.go b/staging/src/k8s.io/api/extensions/v1beta1/types.go index c0ac6fa25dd..70b349f654b 100644 --- a/staging/src/k8s.io/api/extensions/v1beta1/types.go +++ b/staging/src/k8s.io/api/extensions/v1beta1/types.go @@ -1041,10 +1041,10 @@ type NetworkPolicy struct { // +optional Spec NetworkPolicySpec `json:"spec,omitempty" protobuf:"bytes,2,opt,name=spec"` - // Status is the current state of the NetworkPolicy. - // More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status - // +optional - Status NetworkPolicyStatus `json:"status,omitempty" protobuf:"bytes,3,opt,name=status"` + // Status is tombstoned to show why 3 is a reserved protobuf tag. + // This commented field should remain, so in the future if we decide to reimplement + // NetworkPolicyStatus a different protobuf name and tag SHOULD be used! + // Status NetworkPolicyStatus `json:"status,omitempty" protobuf:"bytes,3,opt,name=status"` } // DEPRECATED 1.9 - This group version of PolicyType is deprecated by networking/v1/PolicyType. @@ -1207,48 +1207,6 @@ type NetworkPolicyPeer struct { IPBlock *IPBlock `json:"ipBlock,omitempty" protobuf:"bytes,3,rep,name=ipBlock"` } -// NetworkPolicyConditionType is the type for status conditions on -// a NetworkPolicy. This type should be used with the -// NetworkPolicyStatus.Conditions field. -type NetworkPolicyConditionType string - -const ( - // NetworkPolicyConditionStatusAccepted represents status of a Network Policy that could be properly parsed by - // the Network Policy provider and will be implemented in the cluster - NetworkPolicyConditionStatusAccepted NetworkPolicyConditionType = "Accepted" - - // NetworkPolicyConditionStatusPartialFailure represents status of a Network Policy that could be partially - // parsed by the Network Policy provider and may not be completely implemented due to a lack of a feature or some - // other condition - NetworkPolicyConditionStatusPartialFailure NetworkPolicyConditionType = "PartialFailure" - - // NetworkPolicyConditionStatusFailure represents status of a Network Policy that could not be parsed by the - // Network Policy provider and will not be implemented in the cluster - NetworkPolicyConditionStatusFailure NetworkPolicyConditionType = "Failure" -) - -// NetworkPolicyConditionReason defines the set of reasons that explain why a -// particular NetworkPolicy condition type has been raised. -type NetworkPolicyConditionReason string - -const ( - // NetworkPolicyConditionReasonFeatureNotSupported represents a reason where the Network Policy may not have been - // implemented in the cluster due to a lack of some feature not supported by the Network Policy provider - NetworkPolicyConditionReasonFeatureNotSupported NetworkPolicyConditionReason = "FeatureNotSupported" -) - -// NetworkPolicyStatus describe the current state of the NetworkPolicy. -type NetworkPolicyStatus struct { - // Conditions holds an array of metav1.Condition that describe the state of the NetworkPolicy. - // Current service state - // +optional - // +patchMergeKey=type - // +patchStrategy=merge - // +listType=map - // +listMapKey=type - Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"` -} - // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // +k8s:prerelease-lifecycle-gen:introduced=1.3 // +k8s:prerelease-lifecycle-gen:deprecated=1.9 diff --git a/staging/src/k8s.io/api/extensions/v1beta1/types_test.go b/staging/src/k8s.io/api/extensions/v1beta1/types_test.go new file mode 100644 index 00000000000..70efcf2edc5 --- /dev/null +++ b/staging/src/k8s.io/api/extensions/v1beta1/types_test.go @@ -0,0 +1,42 @@ +/* +Copyright 2023 The Kubernetes 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 v1beta1 + +import ( + "reflect" + "strings" + "testing" +) + +// Test_ServiceSpecRemovedFieldProtobufNumberReservation tests that the reserved protobuf field numbers +// for removed fields are not re-used. DO NOT remove this test for any reason, this ensures that tombstoned +// protobuf field numbers are not accidentally reused by other fields. +func Test_NetworkPolicyRemovedFieldProtobufNumberReservation(t *testing.T) { + obj := reflect.ValueOf(NetworkPolicy{}).Type() + for i := 0; i < obj.NumField(); i++ { + f := obj.Field(i) + protobufSpec := f.Tag.Get("protobuf") + if protobufSpec == "" { + continue + } + + protobufNum := strings.Split(protobufSpec, ",")[1] + if protobufNum == "3" { + t.Errorf("protobuf 3 in NetworkPolicy is reserved for removed status field") + } + } +} diff --git a/staging/src/k8s.io/api/networking/v1/types.go b/staging/src/k8s.io/api/networking/v1/types.go index fa7cf1bd700..a17e2cb5b39 100644 --- a/staging/src/k8s.io/api/networking/v1/types.go +++ b/staging/src/k8s.io/api/networking/v1/types.go @@ -38,10 +38,10 @@ type NetworkPolicy struct { // +optional Spec NetworkPolicySpec `json:"spec,omitempty" protobuf:"bytes,2,opt,name=spec"` - // status represents the current state of the NetworkPolicy. - // More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status - // +optional - Status NetworkPolicyStatus `json:"status,omitempty" protobuf:"bytes,3,opt,name=status"` + // Status is tombstoned to show why 3 is a reserved protobuf tag. + // This commented field should remain, so in the future if we decide to reimplement + // NetworkPolicyStatus a different protobuf name and tag SHOULD be used! + // Status NetworkPolicyStatus `json:"status,omitempty" protobuf:"bytes,3,opt,name=status"` } // PolicyType string describes the NetworkPolicy type @@ -205,48 +205,6 @@ type NetworkPolicyPeer struct { IPBlock *IPBlock `json:"ipBlock,omitempty" protobuf:"bytes,3,rep,name=ipBlock"` } -// NetworkPolicyConditionType is the type for status conditions on -// a NetworkPolicy. This type should be used with the -// NetworkPolicyStatus.Conditions field. -type NetworkPolicyConditionType string - -const ( - // NetworkPolicyConditionStatusAccepted represents status of a Network Policy that could be properly parsed by - // the Network Policy provider and will be implemented in the cluster - NetworkPolicyConditionStatusAccepted NetworkPolicyConditionType = "Accepted" - - // NetworkPolicyConditionStatusPartialFailure represents status of a Network Policy that could be partially - // parsed by the Network Policy provider and may not be completely implemented due to a lack of a feature or some - // other condition - NetworkPolicyConditionStatusPartialFailure NetworkPolicyConditionType = "PartialFailure" - - // NetworkPolicyConditionStatusFailure represents status of a Network Policy that could not be parsed by the - // Network Policy provider and will not be implemented in the cluster - NetworkPolicyConditionStatusFailure NetworkPolicyConditionType = "Failure" -) - -// NetworkPolicyConditionReason defines the set of reasons that explain why a -// particular NetworkPolicy condition type has been raised. -type NetworkPolicyConditionReason string - -const ( - // NetworkPolicyConditionReasonFeatureNotSupported represents a reason where the Network Policy may not have been - // implemented in the cluster due to a lack of some feature not supported by the Network Policy provider - NetworkPolicyConditionReasonFeatureNotSupported NetworkPolicyConditionReason = "FeatureNotSupported" -) - -// NetworkPolicyStatus describes the current state of the NetworkPolicy. -type NetworkPolicyStatus struct { - // conditions holds an array of metav1.Condition that describe the state of the NetworkPolicy. - // Current service state - // +optional - // +patchMergeKey=type - // +patchStrategy=merge - // +listType=map - // +listMapKey=type - Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"` -} - // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // NetworkPolicyList is a list of NetworkPolicy objects. diff --git a/staging/src/k8s.io/api/networking/v1/types_test.go b/staging/src/k8s.io/api/networking/v1/types_test.go new file mode 100644 index 00000000000..40baf566231 --- /dev/null +++ b/staging/src/k8s.io/api/networking/v1/types_test.go @@ -0,0 +1,42 @@ +/* +Copyright 2023 The Kubernetes 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 v1 + +import ( + "reflect" + "strings" + "testing" +) + +// Test_ServiceSpecRemovedFieldProtobufNumberReservation tests that the reserved protobuf field numbers +// for removed fields are not re-used. DO NOT remove this test for any reason, this ensures that tombstoned +// protobuf field numbers are not accidentally reused by other fields. +func Test_NetworkPolicyRemovedFieldProtobufNumberReservation(t *testing.T) { + obj := reflect.ValueOf(NetworkPolicy{}).Type() + for i := 0; i < obj.NumField(); i++ { + f := obj.Field(i) + protobufSpec := f.Tag.Get("protobuf") + if protobufSpec == "" { + continue + } + + protobufNum := strings.Split(protobufSpec, ",")[1] + if protobufNum == "3" { + t.Errorf("protobuf 3 in NetworkPolicy is reserved for removed status field") + } + } +} diff --git a/test/conformance/testdata/ineligible_endpoints.yaml b/test/conformance/testdata/ineligible_endpoints.yaml index 6ee26c48567..1bbf3c2256d 100644 --- a/test/conformance/testdata/ineligible_endpoints.yaml +++ b/test/conformance/testdata/ineligible_endpoints.yaml @@ -238,9 +238,12 @@ - endpoint: logFileListHandler reason: optional feature link: https://github.com/kubernetes/kubernetes/issues/108675 -- endpoint: patchNetworkingV1NamespacedNetworkPolicyStatus - reason: endpoints is currently feature gated and and will only receive e2e & conformance test in 1.25 - link: https://github.com/kubernetes/kubernetes/pull/107963 +- endpoint: readCoreV1NodeStatus + reason: Kubernetes distribution would reasonably not allow this action via the API + link: https://github.com/kubernetes/kubernetes/issues/109379 +- endpoint: replaceCoreV1NodeStatus + reason: Kubernetes distribution would reasonably not allow this action via the API + link: https://github.com/kubernetes/kubernetes/issues/109379 - endpoint: deleteCoreV1CollectionNode reason: Kubernetes distribution would reasonably not allow this action via the API link: https://github.com/kubernetes/kubernetes/issues/109379 diff --git a/test/e2e/network/netpol/network_policy_api.go b/test/e2e/network/netpol/network_policy_api.go index 18fe55c7475..8b9c1557bac 100644 --- a/test/e2e/network/netpol/network_policy_api.go +++ b/test/e2e/network/netpol/network_policy_api.go @@ -269,79 +269,4 @@ var _ = common.SIGDescribe("Netpol API", func() { framework.ExpectNoError(err) framework.ExpectEqual(len(nps.Items), 0, "filtered list should be 0 items") }) - - /* - Release: v1.24 - Testname: NetworkPolicy support status subresource - Description: - - Status condition without a Reason cannot exist - - Status should support conditions - - Two conditions with the same type cannot exist. - */ - ginkgo.It("should support creating NetworkPolicy with Status subresource [Feature:NetworkPolicyStatus]", func(ctx context.Context) { - ns := f.Namespace.Name - npClient := f.ClientSet.NetworkingV1().NetworkPolicies(ns) - - ginkgo.By("NetworkPolicy should deny invalid status condition without Reason field") - - namespaceSelector := &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "ns-name": "pod-b", - }, - } - podSelector := &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "pod-name": "client-a", - }, - } - ingressRule := networkingv1.NetworkPolicyIngressRule{} - ingressRule.From = append(ingressRule.From, networkingv1.NetworkPolicyPeer{PodSelector: podSelector, NamespaceSelector: namespaceSelector}) - - npTemplate := GenNetworkPolicy(SetGenerateName("e2e-example-netpol-status-validate"), - SetObjectMetaLabel(map[string]string{"special-label": f.UniqueName}), - SetSpecPodSelectorMatchLabels(map[string]string{"pod-name": "test-pod"}), - SetSpecIngressRules(ingressRule)) - newNetPol, err := npClient.Create(ctx, npTemplate, metav1.CreateOptions{}) - - framework.ExpectNoError(err, "request template:%v", npTemplate) - - condition := metav1.Condition{ - Type: string(networkingv1.NetworkPolicyConditionStatusAccepted), - Status: metav1.ConditionTrue, - Reason: "RuleApplied", - LastTransitionTime: metav1.Time{Time: time.Now().Add(-5 * time.Minute)}, - Message: "rule was successfully applied", - ObservedGeneration: 2, - } - - status := networkingv1.NetworkPolicyStatus{ - Conditions: []metav1.Condition{ - condition, - }, - } - - ginkgo.By("NetworkPolicy should support valid status condition") - newNetPol.Status = status - - _, err = npClient.UpdateStatus(ctx, newNetPol, metav1.UpdateOptions{}) - framework.ExpectNoError(err, "request template:%v", newNetPol) - - ginkgo.By("NetworkPolicy should not support status condition without reason field") - newNetPol.Status.Conditions[0].Reason = "" - _, err = npClient.UpdateStatus(ctx, newNetPol, metav1.UpdateOptions{}) - framework.ExpectError(err, "request template:%v", newNetPol) - - ginkgo.By("NetworkPolicy should not support status condition with duplicated types") - newNetPol.Status.Conditions = []metav1.Condition{condition, condition} - newNetPol.Status.Conditions[1].Status = metav1.ConditionFalse - _, err = npClient.UpdateStatus(ctx, newNetPol, metav1.UpdateOptions{}) - framework.ExpectError(err, "request template:%v", newNetPol) - - ginkgo.By("deleting all test collection") - err = npClient.DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{LabelSelector: "special-label=" + f.UniqueName}) - framework.ExpectNoError(err) - nps, err := npClient.List(ctx, metav1.ListOptions{LabelSelector: "special-label=" + f.UniqueName}) - framework.ExpectNoError(err) - framework.ExpectEqual(len(nps.Items), 0, "filtered list should be 0 items") - }) }) diff --git a/test/integration/apiserver/apply/reset_fields_test.go b/test/integration/apiserver/apply/reset_fields_test.go index 3da61b61d6b..ba3ed9845f1 100644 --- a/test/integration/apiserver/apply/reset_fields_test.go +++ b/test/integration/apiserver/apply/reset_fields_test.go @@ -30,12 +30,9 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" - featuregatetesting "k8s.io/component-base/featuregate/testing" apiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" - k8sfeatures "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/test/integration/etcd" "k8s.io/kubernetes/test/integration/framework" @@ -55,8 +52,6 @@ var resetFieldsStatusData = map[schema.GroupVersionResource]string{ gvr("extensions", "v1beta1", "ingresses"): `{"status": {"loadBalancer": {"ingress": [{"ip": "127.0.0.2"}]}}}`, gvr("networking.k8s.io", "v1beta1", "ingresses"): `{"status": {"loadBalancer": {"ingress": [{"ip": "127.0.0.2"}]}}}`, gvr("networking.k8s.io", "v1", "ingresses"): `{"status": {"loadBalancer": {"ingress": [{"ip": "127.0.0.2"}]}}}`, - gvr("extensions", "v1beta1", "networkpolicies"): `{"status": {"conditions":[{"type":"Accepted","status":"True","lastTransitionTime":"2020-01-01T00:00:00Z","reason":"RuleApplied","message":"Rule was applied"}]}}`, - gvr("networking.k8s.io", "v1", "networkpolicies"): `{"status": {"conditions":[{"type":"Accepted","status":"True","lastTransitionTime":"2020-01-01T00:00:00Z","reason":"RuleApplied","message":"Rule was applied"}]}}`, gvr("autoscaling", "v1", "horizontalpodautoscalers"): `{"status": {"currentReplicas": 25}}`, gvr("autoscaling", "v2", "horizontalpodautoscalers"): `{"status": {"currentReplicas": 25}}`, gvr("batch", "v1", "cronjobs"): `{"status": {"lastScheduleTime": "2020-01-01T00:00:00Z"}}`, @@ -136,8 +131,6 @@ var resetFieldsSpecData = map[schema.GroupVersionResource]string{ gvr("extensions", "v1beta1", "ingresses"): `{"spec": {"backend": {"serviceName": "service2"}}}`, gvr("networking.k8s.io", "v1beta1", "ingresses"): `{"spec": {"backend": {"serviceName": "service2"}}}`, gvr("networking.k8s.io", "v1", "ingresses"): `{"spec": {"defaultBackend": {"service": {"name": "service2"}}}}`, - gvr("extensions", "v1beta1", "networkpolicies"): `{"spec":{"podSelector":{"matchLabels":{"app":"web"}},"ingress":[]}}`, - gvr("networking.k8s.io", "v1", "networkpolicies"): `{"spec":{"podSelector":{"matchLabels":{"app":"web"}},"ingress":[]}}`, gvr("policy", "v1", "poddisruptionbudgets"): `{"spec": {"selector": {"matchLabels": {"anokkey2": "anokvalue"}}}}`, gvr("policy", "v1beta1", "poddisruptionbudgets"): `{"spec": {"selector": {"matchLabels": {"anokkey2": "anokvalue"}}}}`, gvr("storage.k8s.io", "v1alpha1", "volumeattachments"): `{"metadata": {"name": "vaName2"}, "spec": {"nodeName": "localhost2"}}`, @@ -162,8 +155,6 @@ var resetFieldsSpecData = map[schema.GroupVersionResource]string{ // confirms that the fieldmanager1 is wiped of the status and fieldmanager2 is wiped of the spec. // We then attempt to apply obj2 to the spec endpoint which fails with an expected conflict. func TestApplyResetFields(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, k8sfeatures.NetworkPolicyStatus, true)() - server, err := apiservertesting.StartTestServer(t, apiservertesting.NewDefaultTestServerOptions(), []string{"--disable-admission-plugins", "ServiceAccount,TaintNodesByCondition"}, framework.SharedEtcd()) if err != nil { t.Fatal(err) diff --git a/test/integration/apiserver/apply/status_test.go b/test/integration/apiserver/apply/status_test.go index 76db91fcfd6..21a12f1000a 100644 --- a/test/integration/apiserver/apply/status_test.go +++ b/test/integration/apiserver/apply/status_test.go @@ -45,8 +45,6 @@ var statusData = map[schema.GroupVersionResource]string{ gvr("extensions", "v1beta1", "ingresses"): `{"status": {"loadBalancer": {"ingress": [{"ip": "127.0.0.1"}]}}}`, gvr("networking.k8s.io", "v1beta1", "ingresses"): `{"status": {"loadBalancer": {"ingress": [{"ip": "127.0.0.1"}]}}}`, gvr("networking.k8s.io", "v1", "ingresses"): `{"status": {"loadBalancer": {"ingress": [{"ip": "127.0.0.1"}]}}}`, - gvr("extensions", "v1beta1", "networkpolicies"): `{"status": {"conditions":[{"type":"Accepted","status":"False","lastTransitionTime":"2020-01-01T00:00:00Z","reason":"RuleApplied","message":"Rule was applied"}]}}`, - gvr("networking.k8s.io", "v1", "networkpolicies"): `{"status": {"conditions":[{"type":"Accepted","status":"False","lastTransitionTime":"2020-01-01T00:00:00Z","reason":"RuleApplied","message":"Rule was applied"}]}}`, gvr("autoscaling", "v1", "horizontalpodautoscalers"): `{"status": {"currentReplicas": 5}}`, gvr("autoscaling", "v2", "horizontalpodautoscalers"): `{"status": {"currentReplicas": 5}}`, gvr("batch", "v1", "cronjobs"): `{"status": {"lastScheduleTime": null}}`,