[RFR] Add support for apiPaths #869
Conversation
bsquizz
changed the title
|
LGTM 👍 |
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| for _, apiPath := range apiPaths { |
There was a problem hiding this comment.
Is there a reason not to turn this into a formatted list as you did above and then use that to iterate over?
Another question - should we take the opportunity here to make the apiPaths field require you to put the entire api path, not just the apiPath suffix?
There was a problem hiding this comment.
I just updated it to use the same approach and format the strings first.
Re the second question -- ok, yes we can do that here if you want to. I think I should just be able to use a +kubebuilder:validation:Pattern=<regex pattern> annotation on the field, correct? What type of patterns do we want to permit here to start with? Only /api/<text>/? Technically I think the regex for this will be something like:
//+kubuilder:validation:Pattern="^\/api\/[a-zA-Z-]+\/$"
|
Looks good to me, there is a failing test - I didn't look , but let me know if you need anything in helping debug it. |
psav
added
pr-architectural-change
|
/retest |
|
The assert in I'm not sure how my changes here would have impacted cert auth. We did modify |
|
/retest |
1 similar comment
|
/retest |
|
All checks passing now :) |
|
Thanks for this enhancement! |
Some apps have a need to expose two API paths to the same back-end. This adds support for a new
[]stringfield calledapiPaths-- in 'local' mode, Clowder will now configure theIngressresource it creates with one or more paths.apiPathwill now become deprecated.