Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add namespace scoped ParametersReference to IngressClass #99275

Merged
merged 1 commit into from Mar 6, 2021
Merged

Add namespace scoped ParametersReference to IngressClass #99275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 62 additions & 2 deletions api/openapi-spec/swagger.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 36 additions & 1 deletion pkg/apis/networking/types.go
View file
Open in desktop
Expand Up @@ -313,7 +313,42 @@ type IngressClassSpec struct {
// configuration for the controller. This is optional if the controller does
// not require extra parameters.
// +optional
Parameters *api.TypedLocalObjectReference
Parameters *IngressClassParametersReference
}

const (
// IngressClassParametersReferenceScopeNamespace indicates that the
// referenced Parameters resource is namespace-scoped.
IngressClassParametersReferenceScopeNamespace = "Namespace"
// IngressClassParametersReferenceScopeNamespace indicates that the
// referenced Parameters resource is cluster-scoped.
IngressClassParametersReferenceScopeCluster = "Cluster"
)

// IngressClassParametersReference identifies an API object. This can be used
// to specify a cluster or namespace-scoped resource.
type IngressClassParametersReference struct {
// APIGroup is the group for the resource being referenced. If APIGroup is
// not specified, the specified Kind must be in the core API group. For any
// other third-party types, APIGroup is required.
// +optional
APIGroup *string
// Kind is the type of resource being referenced.
Kind string
// Name is the name of resource being referenced.
Name string
// Scope represents if this refers to a cluster or namespace scoped resource.
// This may be set to "Cluster" (default) or "Namespace".
// Field can be enabled with IngressClassNamespacedParams feature gate.
// +optional
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: We're adding a +featureGate=IngressClassNamespacedParams tag for gated fields, which would be both of these

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A quick grep revealed no prior art.
I added the tag here as well as in k8s.io/api/networking/*. Let me know if we only need this in pkg/apis.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We really only needed in the versioned files, but most people sync those comments to the unversioned

// +featureGate=IngressClassNamespacedParams
Scope *string
// Namespace is the namespace of the resource being referenced. This field is
// required when scope is set to "Namespace" and must be unset when scope is set to
// "Cluster".
// +optional
// +featureGate=IngressClassNamespacedParams
Namespace *string
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
Expand Down
12 changes: 12 additions & 0 deletions pkg/apis/networking/v1/defaults.go
View file
Open in desktop
Expand Up @@ -20,6 +20,9 @@ import (
"k8s.io/api/core/v1"
networkingv1 "k8s.io/api/networking/v1"
"k8s.io/apimachinery/pkg/runtime"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/features"
utilpointer "k8s.io/utils/pointer"
)

func addDefaultingFuncs(scheme *runtime.Scheme) error {
Expand All @@ -43,3 +46,12 @@ func SetDefaults_NetworkPolicy(obj *networkingv1.NetworkPolicy) {
}
}
}

func SetDefaults_IngressClass(obj *networkingv1.IngressClass) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: you could make this named and typed for the Params struct, so you don't need the additional nil-check (I think)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried but this didn't work. I'm not familiar with codegen here but based on some reading of a few zz_generated.defaults.go that's not the case.
Furthermore, scheme.AddTypeDefaultingFunc() requires a type which implements runtime.Object.

if !utilfeature.DefaultFeatureGate.Enabled(features.IngressClassNamespacedParams) {
return
}
if obj.Spec.Parameters != nil && obj.Spec.Parameters.Scope == nil {
obj.Spec.Parameters.Scope = utilpointer.StringPtr(networkingv1.IngressClassParametersReferenceScopeCluster)
}
}
130 changes: 130 additions & 0 deletions pkg/apis/networking/v1/defaults_test.go
View file
Open in desktop
Expand Up @@ -24,10 +24,14 @@ import (
apiequality "k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/kubernetes/pkg/api/legacyscheme"
_ "k8s.io/kubernetes/pkg/apis/core/install"
_ "k8s.io/kubernetes/pkg/apis/networking/install"
. "k8s.io/kubernetes/pkg/apis/networking/v1"
"k8s.io/kubernetes/pkg/features"
utilpointer "k8s.io/utils/pointer"
)

func TestSetDefaultNetworkPolicy(t *testing.T) {
Expand Down Expand Up @@ -234,6 +238,132 @@ func TestSetDefaultNetworkPolicy(t *testing.T) {
}
}

func TestSetDefaultsForIngressClassParametersReference(t *testing.T) {
tests := []struct {
name string
original *networkingv1.IngressClass
expected *networkingv1.IngressClass
enableNamespaceScopedParamsGate bool
}{
{
name: "populated parameters sets the default Scope when feature is enabled",
original: &networkingv1.IngressClass{
Spec: networkingv1.IngressClassSpec{
Controller: "controller",
Parameters: &networkingv1.IngressClassParametersReference{
Kind: "k",
Name: "n",
},
},
},
expected: &networkingv1.IngressClass{
Spec: networkingv1.IngressClassSpec{
Controller: "controller",
Parameters: &networkingv1.IngressClassParametersReference{
Kind: "k",
Name: "n",
Scope: utilpointer.StringPtr(networkingv1.IngressClassParametersReferenceScopeCluster),
},
},
},
enableNamespaceScopedParamsGate: true,
},
{
name: "existing scope is not overridden when feature is enabled",
original: &networkingv1.IngressClass{
Spec: networkingv1.IngressClassSpec{
Controller: "controller",
Parameters: &networkingv1.IngressClassParametersReference{
Kind: "k",
Name: "n",
Scope: utilpointer.StringPtr(networkingv1.IngressClassParametersReferenceScopeNamespace),
Namespace: utilpointer.StringPtr("foo-ns"),
},
},
},
expected: &networkingv1.IngressClass{
Spec: networkingv1.IngressClassSpec{
Controller: "controller",
Parameters: &networkingv1.IngressClassParametersReference{
Kind: "k",
Name: "n",
Scope: utilpointer.StringPtr(networkingv1.IngressClassParametersReferenceScopeNamespace),
Namespace: utilpointer.StringPtr("foo-ns"),
},
},
},
enableNamespaceScopedParamsGate: true,
},
{
name: "empty Parameters does not set the default Scope when feature is enabled",
original: &networkingv1.IngressClass{
Spec: networkingv1.IngressClassSpec{
Controller: "controller",
},
},
expected: &networkingv1.IngressClass{
Spec: networkingv1.IngressClassSpec{
Controller: "controller",
},
},
enableNamespaceScopedParamsGate: true,
},
{
name: "populated parameters does not set the default Scope when feature is disabled",
original: &networkingv1.IngressClass{
Spec: networkingv1.IngressClassSpec{
Controller: "controller",
Parameters: &networkingv1.IngressClassParametersReference{
Kind: "k",
Name: "n",
},
},
},
expected: &networkingv1.IngressClass{
Spec: networkingv1.IngressClassSpec{
Controller: "controller",
Parameters: &networkingv1.IngressClassParametersReference{
Kind: "k",
Name: "n",
},
},
},
enableNamespaceScopedParamsGate: false,
},
{
name: "empty Parameters does not set the default Scope when feature is disabled",
original: &networkingv1.IngressClass{
Spec: networkingv1.IngressClassSpec{
Controller: "controller",
},
},
expected: &networkingv1.IngressClass{
Spec: networkingv1.IngressClassSpec{
Controller: "controller",
},
},
enableNamespaceScopedParamsGate: false,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
original := test.original
expected := test.expected
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.IngressClassNamespacedParams, test.enableNamespaceScopedParamsGate)()
obj2 := roundTrip(t, runtime.Object(original))
got, ok := obj2.(*networkingv1.IngressClass)
if !ok {
t.Errorf("unexpected object: %v", got)
t.FailNow()
}
if !apiequality.Semantic.DeepEqual(got.Spec, expected.Spec) {
t.Errorf("got different than expected\ngot:\n\t%+v\nexpected:\n\t%+v", got.Spec, expected.Spec)
}
})
}
}

func roundTrip(t *testing.T, obj runtime.Object) runtime.Object {
t.Helper()
data, err := runtime.Encode(legacyscheme.Codecs.LegacyCodec(SchemeGroupVersion), obj)
Expand Down
42 changes: 40 additions & 2 deletions pkg/apis/networking/v1/zz_generated.conversion.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions pkg/apis/networking/v1/zz_generated.defaults.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.