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

apis: add GatewayClassConditionReason Unsupported #3048

Merged
merged 5 commits into from
May 10, 2024
Merged

apis: add GatewayClassConditionReason Unsupported #3048

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a2f3dd7
apis: add GatewayClassConditionReason Invalid
mikemorris May 2, 2024
1debac4
doc: clarify expected behavior for bad reference in GatewayClass Para…
mikemorris May 2, 2024
896ad6d
docs: GatewayClass SHOULD be rejected with InvalidParameters reason
mikemorris May 2, 2024
4c83e43
apis: switch from Invalid reason to Unsupported
mikemorris May 2, 2024
bcaef32
fixup codegen and yamllint
mikemorris May 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions apis/v1/gatewayclass_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,10 @@ type GatewayClassSpec struct {
// or an implementation-specific custom resource. The resource can be
// cluster-scoped or namespace-scoped.
//
// If the referent cannot be found, the GatewayClass's "InvalidParameters"
// status condition will be true.
// If the referent cannot be found, refers to an unsupported kind, or when
// the data within that resource is malformed, the GatewayClass will be
mikemorris marked this conversation as resolved.
Show resolved Hide resolved
// rejected with the "Accepted" status condition set to "False" and an
// "InvalidParameters" reason.
//
// A Gateway for this GatewayClass may provide its own `parametersRef`. When both are specified,
// the merging behavior is implementation specific.
Expand Down Expand Up @@ -161,6 +163,7 @@ const (
//
// Possible reasons for this condition to be False are:
//
// * "Invalid"
// * "InvalidParameters"
// * "UnsupportedVersion"
//
Expand All @@ -177,8 +180,13 @@ const (
GatewayClassReasonAccepted GatewayClassConditionReason = "Accepted"

// This reason is used with the "Accepted" condition when the
// GatewayClass was not accepted because the parametersRef field
// was invalid, with more detail in the message.
// GatewayClass was not accepted, with more detail in the message.
GatewayClassReasonInvalid GatewayClassConditionReason = "Invalid"
Copy link
Member

Choose a reason for hiding this comment

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

Since there's not really much to a GatewayClass, it may be slightly more accurate to use "Unsupported" here if custom GatewayClasses just aren't supported. I think either are probably fine here, interested in what others think.

Copy link
Contributor Author

@mikemorris mikemorris May 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'd be happy with Unsupported instead if folks would prefer that!

Copy link
Contributor

@snehachhabria snehachhabria May 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I feel Unsupported is a better than Invalid as its a more accurate reason even if a message is or is not provided. Invalid can have a wider scope.


// This reason is used with the "Accepted" condition when the GatewayClass
// was not accepted because the parametersRef field refers to a nonexistent
// or unsupported resource or kind, or when the data within that resource is
// malformed.
GatewayClassReasonInvalidParameters GatewayClassConditionReason = "InvalidParameters"

// This reason is used with the "Accepted" condition when the
Expand Down