Graduating Ingress to V1 #88041

robscott · 2020-02-11T21:53:47Z

What type of PR is this?
/kind api-change

What this PR does / why we need it:
This graduates Ingress to v1.

Does this PR introduce a user-facing change?:

Ingress has graduated to v1, with new attributes including `pathType`, `class`, and support for new custom Ingress backends. A new IngressClass resource allows for additional configuration of Ingress classes.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Enhancement Issue: Graduate Ingress to V1 enhancements#1453
KEP: https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/20190125-ingress-api-group.md

/cc @bowei @cmluciano
/sig network
/priority important-soon

fejta-bot · 2020-02-11T22:37:35Z

This PR may require API review.

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

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

bowei · 2020-02-12T06:58:51Z

pkg/apis/extensions/v1beta1/defaults.go

@@ -26,6 +26,8 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 )

+var defaultPathType = extensionsv1beta1.PathTypeImplementationSpecific


Can we inline this since we don't have a docstring associated with this?

This pattern actually came from what I was doing for EndpointSlice validation. I was initially using inlined vars here, but Tim recommended moving to this style in that API review. I've grown to appreciate key values like this being highly visible like this, but can switch back to inline if that's preferred.

bowei · 2020-02-12T07:00:43Z

pkg/apis/networking/types.go

+	// given precedence over this field. The controller may emit a warning if
+	// the field and annotation have different values. Implementations of this
+	// API may choose to ignore Ingresses without a class specified.
+	// An IngressClass resource may be marked as default, which can be used to


we should be more specific concerning marking as default. We don't want to be mysterious here, people will have a hard time finding it.

I'm not sure what Bowei's means here but marking an IngressClass as default works only if most of the implementations do respect this. If that's not the case, then you end up in a situation where user expects one thing, based on the k8s core contract but the reality is different.

We should force all controllers to respect it. Maybe this goes into the KEP.

@bowei I have more specific documentation on the IngressClass resource - it was missing in one of the versions when you reviewed this. I was hoping to avoid duplicating that copy on both the Class field and the Class resource, but I can do that if you think it will help make this easier to understand.

@hbagdi This was a last minute addition to the KEP per Tim's suggestion. It mirrors the behavior of StorageClass. An admission controller watches Ingresses and sets a default value for Class if it's not already specified on the Ingress and there is an IngressClass resource that has been marked as default.

I'm not saying the addition doesn't make sense and the behavior is incorrect.

But, this:

Implementations of this API may choose to ignore Ingresses without a class specified.

This seems like a problematic may to me.
We should define the behavior of controllers around what to do when the class is not set at all. If one controller ignores them, and user installs another one, and it takes over those Ingress resources, then that leads to a bad user experience. The guidelines here should be more strict.

Yeah, I agree that it would be nice if we could be more specific here. Unfortunately we're limited to some extent by what already exists. Ingresses have already been valid without a class specified, and we can't make that use case invalid and still maintain backwards compatibility. At the same time, the behavior when this was unspecified was also not specified and therefore was inconsistent, so we're limited with what we can say here without requiring behavior to change dramatically here.

If there were a way to require class to be specified here, that sure would be ideal, but I just don't think we can unfortunately. I'm also open to ideas for how we could further specify how the lack of a class should be handled, I just haven't found anything that works well within the constraints we have (especially backwards compatibility).

However, this is a V1 spec for which (AFAIK) every ingress controller will need code changes to support (even if it is just adding informers for the new types). This seems like the best opportunity to make another incompatible change :)

James, the problem is that Ingress has been v1beta1 for so long that is essentially a GA API, and beta is only in the name. People rely on it like a v1 GA API and breaking the behavior will not go well for most users of the community.

Rob, you have a good point.
I get that we can't enforce things in the API but we can have stricter guidelines.

A solution I can think of is to replace may with should and then detail on the behavior we want implementors to follow.

👍 I like that idea, thanks! I've replaced the may with should and I think that at least gets us closer to the goal here.

pkg/apis/networking/types.go

pkg/apis/networking/validation/validation.go

pkg/registry/networking/ingressclass/strategy.go

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

bowei · 2020-02-12T07:56:05Z

plugin/pkg/admission/ingressclass/admission_test.go

+			classField:      nil,
+			classAnnotation: nil,
+			expectedClass:   nil,
+			expectedError:   errors.NewForbidden(networkingv1.Resource("ingresses"), "testing", errors.NewInternalError(fmt.Errorf("2 default IngressClasses were found, only 1 allowed"))),


tests can become fragile if you test for the error message

That's definitely true, in this case I thought it was worthwhile though. This is an admission controller, and it's important not just that it prevents invalid configuration, but that it includes a clear explanation for why the configuration is invalid. In this case, this is testing an error message that is specific to this admission controller, something that would only change if the admission controller itself changed.

fedebongio · 2020-02-13T21:07:17Z

If there is an specific request for SIG api-machinery to review, please tag me and let us know

hbagdi · 2020-02-13T20:44:02Z

pkg/apis/networking/types.go

+// Spec. The `ingressclass.kubernetes.io/is-default-class` annotation can be
+// used to indicate that an IngressClass should be considered default. When a
+// single IngressClass resource has this annotation set to true, new Ingress
+// resources without a class specified will be assigned this default class.


typo: assigned to this

What should be the behavior when two Ingress classes have this behavior?
As simple as two controllers fight over it sounds good enough.

NVM. Found that we do have validation around this.

Yeah this is all just running through an admission controller that mirrors what was already done for StorageClass. If there are more than 1 IngressClasses marked as default in a cluster, we actually block creation of Ingresses until that has been resolved. I think a link will break when I push again, but this is all in plugin/pkg/admission/ingressclass.

hbagdi · 2020-02-13T20:46:46Z

pkg/apis/networking/types.go

+	// given precedence over this field. The controller may emit a warning if
+	// the field and annotation have different values. Implementations of this
+	// API may choose to ignore Ingresses without a class specified.
+	// An IngressClass resource may be marked as default, which can be used to


I'm not sure what Bowei's means here but marking an IngressClass as default works only if most of the implementations do respect this. If that's not the case, then you end up in a situation where user expects one thing, based on the k8s core contract but the reality is different.

We should force all controllers to respect it. Maybe this goes into the KEP.

hbagdi · 2020-02-13T21:01:17Z

staging/src/k8s.io/api/extensions/v1beta1/types.go

@@ -569,6 +569,19 @@ type IngressList struct {

 // IngressSpec describes the Ingress the user wishes to exist.
 type IngressSpec struct {
+	// Class is the name of the IngressClass cluster resource. The associated


Question for my learning: Why do we add the new fields to older resources here?
Is it for some round-tripping behavior?

We do this in networking.v1beta1 as well. 🤔

Yep, it's a strange requirement to enable round-tripping as I understand it. Definitely makes this PR pretty repetitive.

Thank you!
Just to make sure, if one goes through the upgrade, and the old and new resource both are available, then magically your old resource (extensions.Ingress) have the new fields as well, correct?

I'm not saying this is a problem but just want to understand this internal detail.

k8s-ci-robot · 2020-02-15T01:44:18Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: robscott
To complete the pull request process, please assign liggitt
You can assign the PR to them by writing /assign @liggitt in a comment when ready.

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

Needs approval from an approver in each of these files:

api/OWNERS
hack/OWNERS
pkg/api/OWNERS
pkg/apis/OWNERS
pkg/kubeapiserver/OWNERS
pkg/master/storageversionhashdata/OWNERS
pkg/printers/OWNERS
pkg/registry/OWNERS
plugin/OWNERS
staging/src/k8s.io/api/OWNERS
staging/src/k8s.io/apimachinery/pkg/OWNERS
staging/src/k8s.io/client-go/OWNERS
staging/src/k8s.io/kubectl/OWNERS
test/integration/etcd/OWNERS

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

robscott · 2020-02-19T00:29:28Z

@bowei @hbagdi Thanks for the initial reviews! This is ready for another look whenever you have time.

cmluciano · 2020-02-19T20:28:57Z

/retest

Per the Ingress v1 KEP, we are renaming the backend field in IngressSpec to clarify its intended use. Appropriate defaults are added to ensure backend is converted to the new field type. ingress: validate backend based on schema.GroupVersion Signed-off-by: Christopher M. Luciano <cmluciano@us.ibm.com>

Signed-off-by: Christopher M. Luciano <cmluciano@us.ibm.com>

robscott · 2020-02-20T01:05:34Z

/retest

robscott · 2020-02-20T01:10:11Z

@thockin and @liggitt, whenever you have time, this PR is ready for a full API review. I believe it includes all the changes included in the associated Ingress V1 KEP now. Individual pieces of this enhancement have been reviewed independently in other smaller PRs, but now everything is combined and ready for 1 big review.

/assign @thockin @liggitt

liggitt · 2020-02-20T05:49:20Z

pkg/apis/networking/types.go

 	// +optional
-	Backend *IngressBackend
+	Class *string


other places we have names of referenced objects, we call it <kind>Name, e.g.:

pod.spec.runtimeClassName

pod.spec.priorityClassName

pod.spec.serviceAccountName

pvc.spec.storageClassName

Is there a reason not to call this IngressClassName? That would also avoid potential pitfalls of generated clients in various languages choking on a field name that is a common keyword.

Do we also expect ingress controllers to just watch for Ingress objects that match their class name? If so, we should add fieldSelector support for this (so a controller could list/watch ingresses?fieldSelector=spec.ingressClassName=foo, for example).

To do that:

Make methods in the ingress strategy to:

Build a map of selectable fields for an ingress object (example)

Return that map along with labels for matching field/label selectors (example)

Construct a match predicate from those label/field sets (example)

Assign the match predicate function as the ingress storage PredicateFunc (example)

Call AddFieldLabelConversionFunc for each external version (example), mapping the fieldSelector field names to the internal field names. In this case, the external and shared field names would be identical: metadata.name, metadata.namespace, and spec.ingressClassName (or whatever we end up naming the field)

When that's done, the following should work:

kubectl get ingresses.v1beta1.extensions --field-selector=metadata.name=$myname kubectl get ingresses.v1beta1.extensions --field-selector=metadata.namespace=$mynamespace kubectl get ingresses.v1beta1.extensions --field-selector=spec.ingressClassName=$myclassname kubectl get ingresses.v1beta1.networking.k8s.io --field-selector=metadata.name=$myname kubectl get ingresses.v1beta1.networking.k8s.io --field-selector=metadata.namespace=$mynamespace kubectl get ingresses.v1beta1.networking.k8s.io --field-selector=spec.ingressClassName=$myclassname kubectl get ingresses.v1.networking.k8s.io --field-selector=metadata.name=$myname kubectl get ingresses.v1.networking.k8s.io --field-selector=metadata.namespace=$mynamespace kubectl get ingresses.v1.networking.k8s.io --field-selector=spec.ingressClassName=$myclassname

Agree that ingressClassName works better here, will work on changing that.

The relationship between class name and controllers is not as straightforward as it might seem. The class name here refers to the name of the IngressClass resource, which then specifies the controller name. The idea was to allow for a single controller to implement multiple classes of ingress, such as external and internal load balancers. So in this case, we'd likely want IngressClass to be selectable in this same way. At that point, controllers would watch IngressClass resources with their controller name, then modify their watch on Ingress resources to include any with those class names. Does that sound reasonable?

liggitt · 2020-02-20T05:52:00Z

pkg/apis/networking/types.go

+	// Spec is the desired state of the IngressClass.
+	// More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status
+	// +optional
+	Spec IngressClassSpec


Spec without status is interesting... no other *Class types I see have a spec/status split. Is there a particular future use case you have in mind here?

We originally proposed a status as part of this resource but pulled it for now with the idea that it could be added in the future. We want this resource to mirror the GatewayClass resource as closely as possible, and the status there is still a work in progress. Ideally once that stabilized, we'd bring the same status back into IngressClass. The two major things we want to enable a controller to indicate are:

That it is implementing a given class

That parameters associated with the given class are valid/invalid

That may evolve as the discussion around GatewayClass evolves, but we wanted to leave room for status here as well.

liggitt · 2020-02-20T05:59:06Z

pkg/apis/networking/v1/defaults.go

+
+func SetDefaults_HTTPIngressPath(obj *networkingv1.HTTPIngressPath) {
+	if obj.PathType == nil {
+		obj.PathType = &defaultPathType


Is this the default we want for v1 or do we want to require pathType to be explicit when creating via v1 (no default, make required in validation)?

Although I'd initially been working towards making pathType a required field with no default, we ended up deciding against that to avoid making such a significant change as part of a v1 release. This would just make it more work for end users to move to the v1 API - this field would need to be set multiple times (1 per path) for each Ingress.

I was also hesitant to drag out the Ingress v1 graduation process as the work on Service APIs was gaining momentum. As I understand it, requiring the pathType field would require another beta release in 1.18 before we could reach v1 in 1.19. I think there's relatively minimal value gained by that approach. For more context:

kubernetes/enhancements#1413 (comment)

This would just make it more work for end users to move to the v1 API - this field would need to be set multiple times (1 per path) for each Ingress.

Propagating an ambiguous meaning of path as a default to v1 doesn't seem great.

As I understand it, requiring the pathType field would require another beta release in 1.18 before we could reach v1 in 1.19.

If the v1beta1 versions default it, and v1 does not, then validation can require it without affecting v1beta1 API users. No additional release is required.

liggitt · 2020-02-20T06:02:30Z

test/integration/etcd/data.go

@@ -210,18 +210,29 @@ func GetEtcdStorageDataForNamespace(namespace string) map[schema.GroupVersionRes
 		gvr("extensions", "v1beta1", "ingresses"): {
 			Stub:             `{"metadata": {"name": "ingress1"}, "spec": {"backend": {"serviceName": "service", "servicePort": 5000}}}`,
 			ExpectedEtcdPath: "/registry/ingress/" + namespace + "/ingress1",
-			ExpectedGVK:      gvkP("networking.k8s.io", "v1beta1", "Ingress"),
+			ExpectedGVK:      gvkP("networking.k8s.io", "v1", "Ingress"),


revert this to v1beta1 for 1.18

liggitt · 2020-02-20T06:03:05Z

pkg/kubeapiserver/default_storage_factory_builder.go

@@ -54,7 +54,7 @@ func NewStorageFactoryConfig() *StorageFactoryConfig {

 	resources := []schema.GroupVersionResource{
 		batch.Resource("cronjobs").WithVersion("v1beta1"),
-		networking.Resource("ingresses").WithVersion("v1beta1"),
+		networking.Resource("ingresses").WithVersion("v1"),


This can't change to v1 until v1.19 (open a tracking issue to switch it). Changing it in a single release means writes via a 1.18 server in a skewed multi-API-server cluster will make ingresses unreadable by a 1.17 server that doesn't understand v1. The etcd storage integration test catches this and should change back to expecting v1beta1 to be stored for 1.18.

liggitt · 2020-02-20T06:50:02Z

plugin/pkg/admission/ingressclass/admission.go

+func (a *claimDefaulterPlugin) Admit(ctx context.Context, attr admission.Attributes, o admission.ObjectInterfaces) error {
+	if attr.GetResource().GroupResource() != networkingv1.Resource("ingresses") {
+		fmt.Println(attr.GetResource().GroupResource())
+		fmt.Println(networkingv1.Resource("ingresses"))


drop printlns

liggitt · 2020-02-20T06:50:53Z

plugin/pkg/admission/ingressclass/admission.go

+	ingress, ok := attr.GetObject().(*networkingv1.Ingress)
+	// if we can't convert then we don't handle this object so just return
+	if !ok {
+		return nil


If we get here this is an error we should log (we thought we recognized the resource, but the object type did not match).

That also means there are no integration or e2e tests verifying this works, since all actual invocations of this admission plugin would have returned here

liggitt · 2020-02-20T06:51:44Z

plugin/pkg/admission/ingressclass/admission.go

+// Admit sets the default value of a Ingress's class if the user did not specify
+// a class.
+func (a *claimDefaulterPlugin) Admit(ctx context.Context, attr admission.Attributes, o admission.ObjectInterfaces) error {
+	if attr.GetResource().GroupResource() != networkingv1.Resource("ingresses") {


is this intentionally only setting the class field for requests made to the v1 API? I expected it for requests to the v1beta1 APIs as well

liggitt · 2020-02-20T06:52:01Z

plugin/pkg/admission/ingressclass/admission.go

+		return nil
+	}
+
+	ingress, ok := attr.GetObject().(*networkingv1.Ingress)


admission deals with internal types, so this will be a networking.Ingress object

liggitt · 2020-02-20T07:00:35Z

pkg/registry/networking/ingress/strategy.go

@@ -67,11 +69,14 @@ func (ingressStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Ob



If an update of an ingress with a spec.class value drops the spec.class value in the new incoming object (which a client using an old version of client-go would do), should we preserve the existing value in PrepareForUpdate? alternately, should the ingressclass admission plugin also populate the default class on update for ingresses that don't have it or the class annotation set?

liggitt · 2020-02-20T14:41:25Z

Add deprecation notices to networking.k8s.io/v1beta1 Ingress (and update the deprecation notice in extensions/v1beta1 Ingress to point to v1):

// DEPRECATED - This group version of Ingress is deprecated by networking.k8s.io/v1 Ingress. See the release notes for more information.

And add matching deprecation language to the release-note in the PR description

k8s-ci-robot · 2020-02-21T02:07:59Z

@robscott: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-kubernetes-bazel-test	`8a204b9`	link	`/test pull-kubernetes-bazel-test`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

liggitt · 2020-02-21T05:25:19Z

Talked with @robscott today about the new fields added (pathType and class). When we've promoted objects to v1 in the past, we've done so with the previous release able to fully round-trip all fields. That ensures anyone using the v1 API can use all fields without data loss regardless of skew/rollback scenarios.

By promoting to v1 and adding new fields at the same time, skew/rollback scenarios drop data. We've worked hard to avoid that in stable APIs since 1.14.

Options:

In 1.18, update v1beta1 to add the new fields (ingressClassName, pathType). In 1.19, promote to v1 with no round-trip issues.
In 1.18, promote to v1 despite round-trip issues. I'm not in favor of this.
In 1.18, promote to v1, but gate the new fields and treat them as unstable fields added to a stable version. I don't see a real benefit to this over option 1, since we still wait until 1.19 to get a v1 API that works the way we want.

sftim · 2020-02-24T00:51:52Z

pkg/apis/networking/types.go

+	// Parameters is a link to a resource containing additional configuration
+	// for the controller. This is optional if the controller does not require
+	// extra parameters. Example configuration resources include
+	// `core.ConfigMap` or a controller specific Custom Resource.


nit / query; does this mean:

Suggested change

// `core.ConfigMap` or a controller specific Custom Resource.

// `core.ConfigMap` or a controller specific CustomResourceDefinition.

?

sftim

A set of nits around naming.
I recommend writing TLS instead of SSL. It's true that people often write these terms interchangeably; however, TLS is now about 20 years old; I'd be happy to see “SSL” laid to rest in formal documentation.

Also, there's no such thing as SSL SNI; you need TLS to negotiate it.

sftim · 2020-02-24T00:54:24Z

staging/src/k8s.io/api/networking/v1/generated.proto

+  // +optional
+  repeated string hosts = 1;
+
+  // SecretName is the name of the secret used to terminate SSL traffic on 443.


nit:

Suggested change

// SecretName is the name of the secret used to terminate SSL traffic on 443.

// SecretName is the name of the secret used to terminate TLS traffic on port 443.

sftim · 2020-02-24T00:54:56Z

staging/src/k8s.io/api/networking/v1/generated.proto

+  repeated string hosts = 1;
+
+  // SecretName is the name of the secret used to terminate SSL traffic on 443.
+  // Field is left optional to allow SSL routing based on SNI hostname alone.


Suggested change

// Field is left optional to allow SSL routing based on SNI hostname alone.

// Field is left optional to allow TLS routing based on SNI hostname alone.

SSL doesn't have SNI; TLS does.

sftim · 2020-02-24T00:55:41Z

staging/src/k8s.io/api/networking/v1/generated.proto

@@ -45,6 +75,202 @@ message IPBlock {
  repeated string except = 2;
 }

+// Ingress is a collection of rules that allow inbound connections to reach the
+// endpoints defined by a backend. An Ingress can be configured to give services
+// externally-reachable urls, load balance traffic, terminate SSL, offer name


nit:

Suggested change

// externally-reachable urls, load balance traffic, terminate SSL, offer name

// externally-reachable urls, load balance traffic, terminate TLS, offer name

sftim · 2020-02-24T00:56:15Z

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

+
+// Ingress is a collection of rules that allow inbound connections to reach the
+// endpoints defined by a backend. An Ingress can be configured to give services
+// externally-reachable urls, load balance traffic, terminate SSL, offer name


nit:

Suggested change

// externally-reachable urls, load balance traffic, terminate SSL, offer name

// externally-reachable urls, load balance traffic, terminate TLS, offer name

sftim · 2020-02-24T00:56:43Z

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

+	// Ingress, if left unspecified.
+	// +optional
+	Hosts []string `json:"hosts,omitempty" protobuf:"bytes,1,rep,name=hosts"`
+	// SecretName is the name of the secret used to terminate SSL traffic on 443.


Suggested change

// SecretName is the name of the secret used to terminate SSL traffic on 443.

// SecretName is the name of the secret used to terminate TLS traffic on port 443.

sftim · 2020-02-24T00:57:06Z

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

+	// +optional
+	Hosts []string `json:"hosts,omitempty" protobuf:"bytes,1,rep,name=hosts"`
+	// SecretName is the name of the secret used to terminate SSL traffic on 443.
+	// Field is left optional to allow SSL routing based on SNI hostname alone.


Suggested change

// Field is left optional to allow SSL routing based on SNI hostname alone.

// Field is left optional to allow TLS routing based on SNI hostname alone.

sftim · 2020-02-24T00:58:18Z

api/openapi-spec/swagger.json

@@ -11527,6 +11572,239 @@
      ],
      "type": "object"
    },
+    "io.k8s.api.networking.v1.Ingress": {
+      "description": "Ingress is a collection of rules that allow inbound connections to reach the endpoints defined by a backend. An Ingress can be configured to give services externally-reachable urls, load balance traffic, terminate SSL, offer name based virtual hosting etc.",


Suggested change

"description": "Ingress is a collection of rules that allow inbound connections to reach the endpoints defined by a backend. An Ingress can be configured to give services externally-reachable urls, load balance traffic, terminate SSL, offer name based virtual hosting etc.",

"description": "Ingress is a collection of rules that allow inbound connections to reach the endpoints defined by a backend. An Ingress can be configured to give services externally-reachable urls, load balance traffic, terminate TLS, offer name based virtual hosting etc.",

sftim · 2020-02-24T00:58:56Z

api/openapi-spec/swagger.json

+          "type": "array"
+        },
+        "secretName": {
+          "description": "SecretName is the name of the secret used to terminate SSL traffic on 443. Field is left optional to allow SSL routing based on SNI hostname alone. If the SNI host in a listener conflicts with the \"Host\" header field used by an IngressRule, the SNI host is used for termination and value of the Host header is used for routing.",


Suggested change

"description": "SecretName is the name of the secret used to terminate SSL traffic on 443. Field is left optional to allow SSL routing based on SNI hostname alone. If the SNI host in a listener conflicts with the \"Host\" header field used by an IngressRule, the SNI host is used for termination and value of the Host header is used for routing.",

"description": "SecretName is the name of the secret used to terminate TLS traffic on port 443. Field is left optional to allow TLS routing based on SNI hostname alone. If the SNI host in a listener conflicts with the \"Host\" header field used by an IngressRule, the SNI host is used for termination and value of the Host header is used for routing.",

sftim · 2020-02-24T00:59:48Z

staging/src/k8s.io/api/networking/v1/types_swagger_doc_generated.go

@@ -37,6 +57,115 @@ func (IPBlock) SwaggerDoc() map[string]string {
 	return map_IPBlock
 }

+var map_Ingress = map[string]string{
+	"":         "Ingress is a collection of rules that allow inbound connections to reach the endpoints defined by a backend. An Ingress can be configured to give services externally-reachable urls, load balance traffic, terminate SSL, offer name based virtual hosting etc.",


nit:

Suggested change

"": "Ingress is a collection of rules that allow inbound connections to reach the endpoints defined by a backend. An Ingress can be configured to give services externally-reachable urls, load balance traffic, terminate SSL, offer name based virtual hosting etc.",

"": "Ingress is a collection of rules that allow inbound connections to reach the endpoints defined by a backend. An Ingress can be configured to give services externally-reachable urls, load balance traffic, terminate TLS, offer name based virtual hosting etc.",

sftim · 2020-02-24T01:00:22Z

staging/src/k8s.io/api/networking/v1/types_swagger_doc_generated.go

+var map_IngressTLS = map[string]string{
+	"":           "IngressTLS describes the transport layer security associated with an Ingress.",
+	"hosts":      "Hosts are a list of hosts included in the TLS certificate. The values in this list must match the name/s used in the tlsSecret. Defaults to the wildcard host setting for the loadbalancer controller fulfilling this Ingress, if left unspecified.",
+	"secretName": "SecretName is the name of the secret used to terminate SSL traffic on 443. Field is left optional to allow SSL routing based on SNI hostname alone. If the SNI host in a listener conflicts with the \"Host\" header field used by an IngressRule, the SNI host is used for termination and value of the Host header is used for routing.",


nit:

Suggested change

"secretName": "SecretName is the name of the secret used to terminate SSL traffic on 443. Field is left optional to allow SSL routing based on SNI hostname alone. If the SNI host in a listener conflicts with the \"Host\" header field used by an IngressRule, the SNI host is used for termination and value of the Host header is used for routing.",

"secretName": "SecretName is the name of the secret used to terminate TLS traffic on port 443. Field is left optional to allow TLS routing based on SNI hostname alone. If the SNI host in a listener conflicts with the \"Host\" header field used by an IngressRule, the SNI host is used for termination and value of the Host header is used for routing.",

robscott · 2020-02-25T18:36:18Z

Closing this PR in favor of separate PRs. One for IngressClass has already been created, while @cmluciano is working on another one covering PathType and additional backend types. All the feedback here is being incorporated into those PRs.

/close

k8s-ci-robot · 2020-02-25T18:36:21Z

@robscott: Closed this PR.

In response to this:

Closing this PR in favor of separate PRs. One for IngressClass has already been created, while @cmluciano is working on another one covering PathType and additional backend types. All the feedback here is being incorporated into those PRs.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot requested review from bowei and cmluciano February 11, 2020 21:53

robscott force-pushed the ingressv1 branch from 8bcdcea to 812809a Compare February 12, 2020 01:47

bowei reviewed Feb 12, 2020

View reviewed changes

robscott force-pushed the ingressv1 branch 2 times, most recently from 0464e11 to 618949f Compare February 13, 2020 00:28

k8s-ci-robot added area/kubectl area/test sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Feb 13, 2020

robscott force-pushed the ingressv1 branch from 618949f to f5e8eb6 Compare February 13, 2020 01:51

cmluciano mentioned this pull request Feb 13, 2020

Graduate Ingress to V1 kubernetes/enhancements#1453

Closed

hbagdi reviewed Feb 13, 2020

View reviewed changes

robscott force-pushed the ingressv1 branch 4 times, most recently from 71bf126 to 2c6d391 Compare February 15, 2020 01:42

robscott force-pushed the ingressv1 branch from 2c6d391 to 3e5ea56 Compare February 18, 2020 19:46

robscott force-pushed the ingressv1 branch from 9ac526b to 2a4fb56 Compare February 19, 2020 00:17

Christopher M. Luciano and others added 4 commits February 19, 2020 14:29

Adding Ingress PathType

b90316d

Adding IngressClass for v1 networking API

c6e783e

ingress: allow wildcard hosts in IngressRule

aad171c

Signed-off-by: Christopher M. Luciano <cmluciano@us.ibm.com>

robscott force-pushed the ingressv1 branch from 2a4fb56 to aad171c Compare February 19, 2020 22:40

k8s-ci-robot assigned liggitt and thockin Feb 20, 2020

liggitt reviewed Feb 20, 2020

View reviewed changes

liggitt added the api-review Categorizes an issue or PR as actively needing an API review. label Feb 20, 2020

Updates and fixes in response to PR feedback

8a204b9

robscott force-pushed the ingressv1 branch from 0966475 to 8a204b9 Compare February 21, 2020 01:54

liggitt mentioned this pull request Feb 21, 2020

ingress: rename backend to defaultBackend for ingress v1 #80179

Closed

3 tasks

robscott mentioned this pull request Feb 23, 2020

WIP: Adding Class and PathType to Ingress #88442

Closed

sftim reviewed Feb 24, 2020

View reviewed changes

robscott mentioned this pull request Feb 25, 2020

Adding IngressClass to networking/v1beta1 #88509

Merged

k8s-ci-robot closed this Feb 25, 2020

robscott deleted the ingressv1 branch March 11, 2021 04:55

		@@ -67,11 +69,14 @@ func (ingressStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Ob

	// `core.ConfigMap` or a controller specific Custom Resource.
	// `core.ConfigMap` or a controller specific CustomResourceDefinition.

	// SecretName is the name of the secret used to terminate SSL traffic on 443.
	// SecretName is the name of the secret used to terminate TLS traffic on port 443.

	// Field is left optional to allow SSL routing based on SNI hostname alone.
	// Field is left optional to allow TLS routing based on SNI hostname alone.

	// externally-reachable urls, load balance traffic, terminate SSL, offer name
	// externally-reachable urls, load balance traffic, terminate TLS, offer name

	"description": "Ingress is a collection of rules that allow inbound connections to reach the endpoints defined by a backend. An Ingress can be configured to give services externally-reachable urls, load balance traffic, terminate SSL, offer name based virtual hosting etc.",
	"description": "Ingress is a collection of rules that allow inbound connections to reach the endpoints defined by a backend. An Ingress can be configured to give services externally-reachable urls, load balance traffic, terminate TLS, offer name based virtual hosting etc.",

	"description": "SecretName is the name of the secret used to terminate SSL traffic on 443. Field is left optional to allow SSL routing based on SNI hostname alone. If the SNI host in a listener conflicts with the \"Host\" header field used by an IngressRule, the SNI host is used for termination and value of the Host header is used for routing.",
	"description": "SecretName is the name of the secret used to terminate TLS traffic on port 443. Field is left optional to allow TLS routing based on SNI hostname alone. If the SNI host in a listener conflicts with the \"Host\" header field used by an IngressRule, the SNI host is used for termination and value of the Host header is used for routing.",

	"": "Ingress is a collection of rules that allow inbound connections to reach the endpoints defined by a backend. An Ingress can be configured to give services externally-reachable urls, load balance traffic, terminate SSL, offer name based virtual hosting etc.",
	"": "Ingress is a collection of rules that allow inbound connections to reach the endpoints defined by a backend. An Ingress can be configured to give services externally-reachable urls, load balance traffic, terminate TLS, offer name based virtual hosting etc.",

	"secretName": "SecretName is the name of the secret used to terminate SSL traffic on 443. Field is left optional to allow SSL routing based on SNI hostname alone. If the SNI host in a listener conflicts with the \"Host\" header field used by an IngressRule, the SNI host is used for termination and value of the Host header is used for routing.",
	"secretName": "SecretName is the name of the secret used to terminate TLS traffic on port 443. Field is left optional to allow TLS routing based on SNI hostname alone. If the SNI host in a listener conflicts with the \"Host\" header field used by an IngressRule, the SNI host is used for termination and value of the Host header is used for routing.",

Graduating Ingress to V1 #88041

Graduating Ingress to V1 #88041

Conversation

robscott commented Feb 11, 2020 • edited Loading

fejta-bot commented Feb 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fedebongio commented Feb 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented Feb 15, 2020

robscott commented Feb 19, 2020

cmluciano commented Feb 19, 2020

robscott commented Feb 20, 2020

robscott commented Feb 20, 2020

Choose a reason for hiding this comment

liggitt Feb 20, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt Feb 20, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt Feb 20, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt commented Feb 20, 2020

k8s-ci-robot commented Feb 21, 2020 • edited Loading

liggitt commented Feb 21, 2020

Choose a reason for hiding this comment

sftim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robscott commented Feb 25, 2020

k8s-ci-robot commented Feb 25, 2020

robscott commented Feb 11, 2020 •

edited

Loading

liggitt Feb 20, 2020 •

edited

Loading

liggitt Feb 20, 2020 •

edited

Loading

liggitt Feb 20, 2020 •

edited

Loading

k8s-ci-robot commented Feb 21, 2020 •

edited

Loading