Delete RouteObjectReference; make group optional; add godoc #111

Miciah · 2020-02-26T20:23:08Z

Follow-up to #73.

Related to #83, which discusses the relationship between gateways and routes.

Related to #109, which discusses inferring group and resource in references.

Delete `RouteObjectReference`

Delete the unused RouteObjectReference type.

Delete +protobuf=false markers on type aliases for LocalObjectReference as the markers now cause go-to-protobuf to fail with "LocalObjectReference" is not defined.

Follow-up to #12.

api/v1alpha1/gateway_types.go (CertificateObjectReference, ListenerExtensionObjectReference): Delete +protobuf=false markers.
(RouteObjectReference): Delete definition.
api/v1alpha1/gatewayclass_types.go (GatewayClassParametersObjectReference):
api/v1alpha1/httproute_types.go (RouteMatchExtensionObjectReference, RouteFilterExtensionObjectReference, RouteActionTargetObjectReference, RouteActionExtensionObjectReference, RouteHostExtensionObjectReference): Delete +protobuf=false markers.
api/v1alpha1/generated.proto: Regenerate.

Make group optional in object references

Allow omitting the group specification in object references, and document default values for group and resource.

Before this commit, the group could be the empty string but could not be omitted (that is, left unspecified in json or yaml definitions).

api/v1alpha1/gateway_types.go (Listener): Document the default values for group and resource for the listener extension object reference (the defaults are core and configmaps, respectively).
(ListenerTLS): Document the default values for group and resource for certificate object references (the defaults are core and secrets, respectively).
(LocalObjectReference): Make group optional.
api/v1alpha1/gatewayclass_types.go (GatewayClassSpec): Document the default values for group and resource for the parameters object reference (the defaults are core and configmaps, respectively).
api/v1alpha1/httproute_types.go (HTTPRouteHost): Document the default values for group and resource for the extension object reference (the defaults are core and configmaps, respectively).
(HTTPRouteMatch): Document the default values for group and resource for the extension object reference (the defaults are core and configmaps, respectively).
(HTTPRouteFilter): Document the default values for group and resource for the extension object reference (the defaults are core and configmaps, respectively).
(HTTPRouteAction): Document the default values for group and resource for the target object reference (for which the defaults are core and services, respectively) and for the extension object reference (for which the defaults are core and configmaps, respectively).
api/v1alpha1/generated.proto:
config/crd/bases/networking.x-k8s.io_gatewayclasses.yaml:
config/crd/bases/networking.x-k8s.io_gateways.yaml:
config/crd/bases/networking.x-k8s.io_httproutes.yaml: Regenerate.

Document behavior for missing referents

For each API field that specifies an object reference, document the expected behavior when the field's referent cannot be found.

api/v1alpha1/gateway_types.go (GatewaySpec, Listener, ListenerTLS):
api/v1alpha1/gatewayclass_types.go (GatewayClassSpec):
api/v1alpha1/httproute_types.go (HTTPRouteHost, HTTPRouteMatch, HTTPRouteFilter, HTTPRouteAction): Document behavior for missing referents.
api/v1alpha1/generated.proto:
config/crd/bases/networking.x-k8s.io_gatewayclasses.yaml:
config/crd/bases/networking.x-k8s.io_gateways.yaml:
config/crd/bases/networking.x-k8s.io_httproutes.yaml: Regenerate.

Add examples to `LocalObjectReference`

Add an example reference to a service object and an example reference to a custom resource.

api/v1alpha1/gateway_types.go (LocalObjectReference): Add example reference specifications.
api/v1alpha1/generated.proto:
config/crd/bases/networking.x-k8s.io_gatewayclasses.yaml:
config/crd/bases/networking.x-k8s.io_gateways.yaml:
config/crd/bases/networking.x-k8s.io_httproutes.yaml: Regenerate.

Use `fooRef` naming convention for object references

Rename fields that are object references in order to follow the community API conventions.

api/v1alpha1/gateway_types.go (Listener): Rename "extension" field to "extensionRef".
(ListenerTLS): Rename "certificates" field to "certificateRefs".
api/v1alpha1/gatewayclass_types.go (GatewayClassSpec): Rename "parameters" field to "parametersRef".
api/v1alpha1/httproute_types.go (HTTPRouteHost, HTTPRouteMatch, HTTPRouteFilter, HTTPRouteAction): Rename "extension" fields to "extensionRef".
(HTTPRouteAction): Rename "forwardTo" field to "forwardTargetRef".
(HTTPRouteStatus): Rename "gateways" field to "gatewayRefs".
api/v1alpha1/generated.pb.go:
api/v1alpha1/generated.proto:
api/v1alpha1/zz_generated.deepcopy.go:
config/crd/bases/networking.x-k8s.io_gatewayclasses.yaml:
config/crd/bases/networking.x-k8s.io_gateways.yaml:
config/crd/bases/networking.x-k8s.io_httproutes.yaml: Regenerate.

k8s-ci-robot · 2020-02-26T20:23:17Z

Hi @Miciah. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

danehans · 2020-02-26T20:27:45Z

/assign @bowei

danehans · 2020-02-26T20:50:24Z

@Miciah FYI #110 generates the deepcopy funcs that you included in this PR.

danehans · 2020-02-26T20:55:04Z

@Miciah can you run make install to update the CRDs?

bowei · 2020-02-26T23:05:49Z

/ok-to-test

danehans · 2020-02-26T23:13:02Z

@Miciah if #111 merges, status condition references need to be updated.

danehans · 2020-02-26T23:13:59Z

@Miciah #110 merged, so pls remove the generated deepcopy func changes.

Miciah · 2020-02-26T23:44:48Z

Rebased for #110.

Miciah · 2020-02-28T05:05:04Z

Rebased for #30.

Miciah · 2020-02-28T05:11:02Z

api/v1alpha1/httproute_types.go

@@ -140,7 +140,6 @@ type HTTPRouteMatch struct {
 // within a known namespace.
 //
 // +k8s:deepcopy-gen=false
-// +protobuf=false


@howardjohn, I had to delete this specific marker or else go-to-protobuf failed with the following output:

topological order io topological order bytes topological order go/ast topological order encoding/base64 topological order strings topological order k8s.io/apimachinery/pkg/fields topological order sync topological order go/token topological order k8s.io/apimachinery/pkg/watch topological order k8s.io/apimachinery/pkg/selection topological order k8s.io/apimachinery/pkg/conversion/queryparams topological order strconv topological order github.com/google/gofuzz topological order go/parser topological order reflect topological order sync/atomic topological order encoding/json topological order github.com/gogo/protobuf/sortkeys topological order fmt topological order k8s.io/apimachinery/pkg/util/runtime topological order k8s.io/apimachinery/pkg/util/naming topological order time topological order k8s.io/apimachinery/pkg/util/json topological order github.com/gogo/protobuf/proto topological order k8s.io/apimachinery/pkg/api/resource topological order net/url topological order k8s.io/apimachinery/pkg/types topological order k8s.io/klog topological order k8s.io/apimachinery/pkg/util/errors topological order k8s.io/apimachinery/pkg/conversion topological order math/bits topological order errors topological order go/doc topological order math topological order k8s.io/apimachinery/pkg/runtime/schema topological order k8s.io/apimachinery/pkg/util/intstr topological order unsafe topological order k8s.io/apimachinery/pkg/labels topological order os topological order k8s.io/apimachinery/pkg/util/sets topological order k8s.io/apimachinery/pkg/runtime topological order k8s.io/apimachinery/pkg/apis/meta/v1 topological order sigs.k8s.io/controller-runtime/pkg/scheme topological order k8s.io/api/core/v1 topological order sigs.k8s.io/service-apis/api/v1alpha1 2020/02/27 23:33:32 sigs.k8s.io/service-apis/api/v1alpha1/generated.proto:142:12: "LocalObjectReference" is not defined. sigs.k8s.io/service-apis/api/v1alpha1/generated.proto:300:12: "LocalObjectReference" is not defined. sigs.k8s.io/service-apis/api/v1alpha1/generated.proto:312:12: "LocalObjectReference" is not defined. sigs.k8s.io/service-apis/api/v1alpha1/generated.proto:334:12: "LocalObjectReference" is not defined. sigs.k8s.io/service-apis/api/v1alpha1/generated.proto:371:12: "LocalObjectReference" is not defined. sigs.k8s.io/service-apis/api/v1alpha1/generated.proto:424:12: "LocalObjectReference" is not defined. sigs.k8s.io/service-apis/api/v1alpha1/generated.proto:513:12: "LocalObjectReference" is not defined. sigs.k8s.io/service-apis/api/v1alpha1/generated.proto:586:12: "LocalObjectReference" is not defined. sigs.k8s.io/service-apis/api/v1alpha1/generated.proto: warning: Import sigs.k8s.io/controller-runtime/pkg/scheme/generated.proto but not used. sigs.k8s.io/service-apis/api/v1alpha1/generated.proto: warning: Import k8s.io/apimachinery/pkg/runtime/schema/generated.proto but not used. sigs.k8s.io/service-apis/api/v1alpha1/generated.proto: warning: Import k8s.io/api/core/v1/generated.proto but not used. 2020/02/27 23:33:32 protoc -I . -I /home/mmasters/src -I /home/mmasters/src/sigs.k8s.io/service-apis/vendor -I /home/mmasters/src/sigs.k8s.io/service-apis/third_party/protobuf --gogo_out=/home/mmasters/src /home/mmasters/src/sigs.k8s.io/service-apis/api/v1alpha1/generated.proto 2020/02/27 23:33:32 Unable to generate protoc on k8s.io.service_apis.api.v1alpha1: exit status 1 exit status 1

I have not tried using the container image, but I cannot see how that would make a difference. Anyway, go-to-protobuf was happy whether I deleted this marker or all of them, so I deleted all of them.

Ok as long as it builds it should be good, maybe your change fixed the issue we were working around. I am not sure the CI would fail if it doesn't build though, maybe be worth running a quick go build ./... ?

go build ./... succeeded (no output; exit status 0).

Miciah · 2020-04-16T00:04:19Z

Rebased. Latest push also deletes RouteObjectReference as it is no longer used, renames fields that are object references to follow the "fooRef" naming convention, and adds some examples and other minor improvements.

danehans · 2020-04-16T04:29:57Z

api/v1alpha1/gateway_types.go

-	// the empty string for the group) or an implementation-defined resource
-	// (for example, resource "mylistener" in group "networking.acme.io").
+	// ExtensionRef for this Listener.  The resource may be "configmaps"
+	// (omit or use the empty string for the group) or an


@Miciah IMO this is easier to read when "(omit or use the empty string for the group)" is removed.

danehans · 2020-04-16T04:35:41Z

api/v1alpha1/generated.proto

-  // the empty string for the group) or an implementation-defined resource
-  // (for example, resource "mylistener" in group "networking.acme.io").
+  // ExtensionRef for this Listener.  The resource may be "configmaps"
+  // (omit or use the empty string for the group) or an


@Miciah same as https://github.com/kubernetes-sigs/service-apis/pull/111/files#r409274838

Regenerated. Thanks!

danehans · 2020-04-16T04:36:34Z

Other than a nit to improve the ExtensionRef godocs, LGTM.

danehans · 2020-04-16T16:36:22Z

/lgtm

robscott · 2020-04-16T17:12:34Z

/lgtm
/assign @bowei

Delete the unused RouteObjectReference type. Delete +protobuf=false markers on type aliases for LocalObjectReference as the markers now cause go-to-protobuf to fail with '"LocalObjectReference" is not defined'. Follow-up to commit 8dcf12b. * api/v1alpha1/gateway_types.go (CertificateObjectReference) (ListenerExtensionObjectReference): Delete +protobuf=false markers. (RouteObjectReference): Delete definition. * api/v1alpha1/gatewayclass_types.go (GatewayClassParametersObjectReference): * api/v1alpha1/httproute_types.go (RouteMatchExtensionObjectReference) (RouteFilterExtensionObjectReference, RouteActionTargetObjectReference) (RouteActionExtensionObjectReference, RouteHostExtensionObjectReference): Delete +protobuf=false markers. * api/v1alpha1/generated.proto: Regenerate.

Allow omitting the group specification in object references, and document default values for group and resource. Before this commit, the group could be the empty string but could not be omitted (that is, left unspecified in json or yaml definitions). * api/v1alpha1/gateway_types.go (Listener): Document the default values for group and resource for the listener extension object reference (the defaults are core and configmaps, respectively). (ListenerTLS): Document the default values for group and resource for certificate object references (the defaults are core and secrets, respectively). (LocalObjectReference): Make group optional. * api/v1alpha1/gatewayclass_types.go (GatewayClassSpec): Document the default values for group and resource for the parameters object reference (the defaults are core and configmaps, respectively). * api/v1alpha1/httproute_types.go (HTTPRouteHost): Document the default values for group and resource for the extension object reference (the defaults are core and configmaps, respectively). (HTTPRouteMatch): Document the default values for group and resource for the extension object reference (the defaults are core and configmaps, respectively). (HTTPRouteFilter): Document the default values for group and resource for the extension object reference (the defaults are core and configmaps, respectively). (HTTPRouteAction): Document the default values for group and resource for the target object reference (for which the defaults are core and services, respectively) and for the extension object reference (for which the defaults are core and configmaps, respectively). * api/v1alpha1/generated.proto: * config/crd/bases/networking.x-k8s.io_gatewayclasses.yaml: * config/crd/bases/networking.x-k8s.io_gateways.yaml: * config/crd/bases/networking.x-k8s.io_httproutes.yaml: Regenerate.

For each API field that specifies an object reference, document the expected behavior when the field's referent cannot be found. * api/v1alpha1/gateway_types.go (GatewaySpec, Listener, ListenerTLS): * api/v1alpha1/gatewayclass_types.go (GatewayClassSpec): * api/v1alpha1/httproute_types.go (HTTPRouteHost, HTTPRouteMatch) (HTTPRouteFilter, HTTPRouteAction): Document behavior for missing referents. * api/v1alpha1/generated.proto: * config/crd/bases/networking.x-k8s.io_gatewayclasses.yaml: * config/crd/bases/networking.x-k8s.io_gateways.yaml: * config/crd/bases/networking.x-k8s.io_httproutes.yaml: Regenerate.

Add an example reference to a service object and an example reference to a custom resource. * api/v1alpha1/gateway_types.go (LocalObjectReference): Add example reference specifications. * api/v1alpha1/generated.proto: * config/crd/bases/networking.x-k8s.io_gatewayclasses.yaml: * config/crd/bases/networking.x-k8s.io_gateways.yaml: * config/crd/bases/networking.x-k8s.io_httproutes.yaml: Regenerate.

Rename fields that are object references in order to follow the community API conventions. * api/v1alpha1/gateway_types.go (Listener): Rename "extension" field to "extensionRef". (ListenerTLS): Rename "certificates" field to "certificateRefs". * api/v1alpha1/gatewayclass_types.go (GatewayClassSpec): Rename "parameters" field to "parametersRef". * api/v1alpha1/httproute_types.go (HTTPRouteHost, HTTPRouteMatch) (HTTPRouteFilter, HTTPRouteAction): Rename "extension" fields to "extensionRef". (HTTPRouteAction): Rename "forwardTo" field to "forwardTargetRef". (HTTPRouteStatus): Rename "gateways" field to "gatewayRefs". * api/v1alpha1/generated.pb.go: * api/v1alpha1/generated.proto: * api/v1alpha1/zz_generated.deepcopy.go: * config/crd/bases/networking.x-k8s.io_gatewayclasses.yaml: * config/crd/bases/networking.x-k8s.io_gateways.yaml: * config/crd/bases/networking.x-k8s.io_httproutes.yaml: Regenerate.

Miciah · 2020-04-22T22:57:18Z

Rebased; dropped "Regenerate CRDs with new group name" (superseded by #155).

robscott · 2020-04-22T23:25:18Z

Thanks!

/lgtm

bowei · 2020-04-23T17:49:10Z

/lgtm
/approve

k8s-ci-robot · 2020-04-23T17:49:20Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bowei, Miciah

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [bowei]

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

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 26, 2020

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 26, 2020

k8s-ci-robot requested review from bowei and thockin February 26, 2020 20:23

k8s-ci-robot assigned bowei Feb 26, 2020

danehans mentioned this pull request Feb 26, 2020

Updates and Includes Example Resources in Install #112

Merged

Miciah force-pushed the add-namespace-to-route-object-reference branch from 89212ae to 94c34a7 Compare February 26, 2020 23:44

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 26, 2020

Miciah mentioned this pull request Feb 27, 2020

Automated protobuf generation #30

Merged

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 27, 2020

youngnick mentioned this pull request Feb 28, 2020

Document security models #115

Closed

Miciah force-pushed the add-namespace-to-route-object-reference branch from 94c34a7 to 2911ad9 Compare February 28, 2020 05:04

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 28, 2020

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 28, 2020

Miciah commented Feb 28, 2020

View reviewed changes

Miciah force-pushed the add-namespace-to-route-object-reference branch from 2911ad9 to 1f6da51 Compare February 28, 2020 05:24

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 29, 2020

Miciah force-pushed the add-namespace-to-route-object-reference branch from 192a970 to 809a65d Compare April 15, 2020 23:52

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 15, 2020

Miciah force-pushed the add-namespace-to-route-object-reference branch from 809a65d to c002ee4 Compare April 15, 2020 23:58

Miciah changed the title ~~Add namespace to route reference; make group optional; add godoc~~ Delete RouteObjectReference; make group optional; add godoc Apr 16, 2020

danehans reviewed Apr 16, 2020

View reviewed changes

Miciah force-pushed the add-namespace-to-route-object-reference branch from c002ee4 to d056a47 Compare April 16, 2020 16:14

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 16, 2020

k8s-ci-robot assigned robscott Apr 16, 2020

Miciah added 5 commits April 22, 2020 18:55

Miciah force-pushed the add-namespace-to-route-object-reference branch from d056a47 to bb8f995 Compare April 22, 2020 22:56

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 22, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 22, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 23, 2020

k8s-ci-robot merged commit cc82aa7 into kubernetes-sigs:master Apr 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delete RouteObjectReference; make group optional; add godoc #111

Delete RouteObjectReference; make group optional; add godoc #111

Miciah commented Feb 26, 2020 •

edited

Loading

k8s-ci-robot commented Feb 26, 2020

danehans commented Feb 26, 2020

danehans commented Feb 26, 2020

danehans commented Feb 26, 2020

bowei commented Feb 26, 2020

danehans commented Feb 26, 2020

danehans commented Feb 26, 2020

Miciah commented Feb 26, 2020

Miciah commented Feb 28, 2020

Miciah Feb 28, 2020

howardjohn Feb 28, 2020

Miciah Feb 28, 2020

Miciah commented Apr 16, 2020

danehans Apr 16, 2020

Miciah Apr 16, 2020

danehans Apr 16, 2020

Miciah Apr 16, 2020

danehans commented Apr 16, 2020

danehans commented Apr 16, 2020

robscott commented Apr 16, 2020

Miciah commented Apr 22, 2020

robscott commented Apr 22, 2020

bowei commented Apr 23, 2020

k8s-ci-robot commented Apr 23, 2020

Delete RouteObjectReference; make group optional; add godoc #111

Delete RouteObjectReference; make group optional; add godoc #111

Conversation

Miciah commented Feb 26, 2020 • edited Loading

Delete RouteObjectReference

Make group optional in object references

Document behavior for missing referents

Add examples to LocalObjectReference

Use fooRef naming convention for object references

k8s-ci-robot commented Feb 26, 2020

danehans commented Feb 26, 2020

danehans commented Feb 26, 2020

danehans commented Feb 26, 2020

bowei commented Feb 26, 2020

danehans commented Feb 26, 2020

danehans commented Feb 26, 2020

Miciah commented Feb 26, 2020

Miciah commented Feb 28, 2020

Miciah Feb 28, 2020

Choose a reason for hiding this comment

howardjohn Feb 28, 2020

Choose a reason for hiding this comment

Miciah Feb 28, 2020

Choose a reason for hiding this comment

Miciah commented Apr 16, 2020

danehans Apr 16, 2020

Choose a reason for hiding this comment

Miciah Apr 16, 2020

Choose a reason for hiding this comment

danehans Apr 16, 2020

Choose a reason for hiding this comment

Miciah Apr 16, 2020

Choose a reason for hiding this comment

danehans commented Apr 16, 2020

danehans commented Apr 16, 2020

robscott commented Apr 16, 2020

Miciah commented Apr 22, 2020

robscott commented Apr 22, 2020

bowei commented Apr 23, 2020

k8s-ci-robot commented Apr 23, 2020

Miciah commented Feb 26, 2020 •

edited

Loading

Delete `RouteObjectReference`

Add examples to `LocalObjectReference`

Use `fooRef` naming convention for object references