-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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.
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
Add validation for apigw creation with no routes #16649
Conversation
@@ -769,6 +770,9 @@ func (e *APIGatewayConfigEntry) Validate() error { | |||
return err | |||
} | |||
|
|||
if len(e.Listeners) == 0 { | |||
return errors.New("api gateway must have at least one listener") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor (and more stylistic than anything and not blocking), would we want to use a sentinel error here so callers could act on the error type in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I don't feel like that matches the way the other validations are handled, but I'm also realizing I didn't follow that way either. Ex: https://github.com/hashicorp/consul/blob/main/agent/structs/config_entry_gateways.go#L803
I do think that is probably a good idea for the future though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Especially because I think it sends a 500 error back right now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! Would you mind just formatting the .mdx
tables you added so the vertical bars align?
Description
To meet the standard (Gateways require at least one listener defined) we need to add checks in to verify that at least one listener is defined.
Testing & Reproduction steps
Open another terminal, create a file called gateway.hcl with the following content:
After creating the file, run the following command in the same terminal:
This should error out, saying "api gateway must have at least one listener"
PR Checklist