Skip to content

docs: setting proper reason for Gateway parametersRef Accepted condition #3579

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
Merged
merged 1 commit into from
Jan 31, 2025
Merged

docs: setting proper reason for Gateway parametersRef Accepted condition #3579

merged 1 commit into from
Jan 31, 2025

Conversation

mlavacca
Copy link
Member

What type of PR is this?

/kind documentation

What this PR does / why we need it:

For the Gateway infrastructure stanza, the InvalidParameters reason SHOULD be used with the Accepted condition in case the object referenced does not exist, is of an unsupported kind, or is malformed.

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

For the Gateway infrastructure stanza, the `InvalidParameters` reason SHOULD be used with the `Accepted` condition in case the object referenced does not exist, is of an unsupported kind, or is malformed.

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/documentation Categorizes issue or PR as related to documentation. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 31, 2025
@mlavacca mlavacca changed the title docs: setting proper reason for Gateway parametersRef Acceptedcondition docs: setting proper reason for Gateway parametersRef Accepted condition Jan 31, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mlavacca

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:

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 k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 31, 2025
@mlavacca
docs: Gateway parametersRef Accepted condition
1aab8c4 
The `InvalidParameters` reason SHOULD be used for the `Accepted`
condition in case the object referenced does not exist, is of an
unsupported Kind, or is malformed.

Signed-off-by: Mattia Lavacca <[email protected]>
@mlavacca mlavacca force-pushed the gateway-invalidparameters-reason branch from b063678 to 1aab8c4 Compare January 31, 2025 16:21
@robscott
Copy link
Member

Thanks @mlavacca!

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 31, 2025
@k8s-ci-robot k8s-ci-robot merged commit 56a2e7e into kubernetes-sigs:main Jan 31, 2025
13 checks passed
sjberman
sjberman reviewed Mar 25, 2025
View reviewed changes
apis/v1/gateway_types.go
Comment on lines +791 to +794
// If the referent cannot be found, refers to an unsupported kind, or when
// the data within that resource is malformed, the Gateway SHOULD be
// rejected with the "Accepted" status condition set to "False" and an
// "InvalidParameters" reason.
Copy link
Contributor

@sjberman sjberman Mar 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@mlavacca

Sorry, seeing this late after reviewing the release notes.

So with this suggestion, there could be a case where a user has a gateway proxy provisioned and running due to a valid Gateway resource, then makes a change to the parametersRef resource that is invalid, and now the Gateway could be marked as invalid and the entire Route tree below it is nullified (and depending on the implementation, the proxy could be deprovisioned because the Gateway is no longer accepted). That's pretty disruptive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjberman sjberman sjberman left review comments

@robscott robscott Awaiting requested review from robscott

@shaneutt shaneutt Awaiting requested review from shaneutt

Assignees

@robscott robscott

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/documentation Categorizes issue or PR as related to documentation. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mlavacca @k8s-ci-robot @robscott @sjberman