-
Notifications
You must be signed in to change notification settings - Fork 271
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
feat: operator creates app and UI Ingresses #5264
Conversation
Thank you for creating a pull request! Pinging @EricWittmann to respond or triage. |
...ntroller/src/main/java/io/apicurio/registry/operator/resource/ui/UIIngressDiscriminator.java
Outdated
Show resolved
Hide resolved
...controller/src/main/java/io/apicurio/registry/operator/resource/ui/UIDeploymentResource.java
Outdated
Show resolved
Hide resolved
operator/controller/src/main/java/io/apicurio/registry/operator/util/BeanUtil.java
Outdated
Show resolved
Hide resolved
operator/controller/src/main/java/io/apicurio/registry/operator/util/HostUtil.java
Outdated
Show resolved
Hide resolved
operator/controller/src/main/java/io/apicurio/registry/operator/util/HostUtil.java
Outdated
Show resolved
Hide resolved
@@ -40,5 +44,16 @@ void demoDeployment() { | |||
.getSpec().getClusterIP()).isNotBlank(); | |||
return true; | |||
}); | |||
|
|||
// Ingresses | |||
await().ignoreExceptions().until(() -> { |
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.
Since now, we have access to both the API and the UI, can you please, add an integration test to access those?
operator/controller/src/main/java/io/apicurio/registry/operator/util/IngressUtil.java
Outdated
Show resolved
Hide resolved
Forgot to mention, we also need a top level CR field to disable the default ingress creation for the case where someone wants a highly customized one. It would be nice in this PR, but can also be done as a follow up. 👍 |
Agreed, I'll do it in the next PR. |
private Util() { | ||
} | ||
|
||
public static boolean isBlank(String value) { |
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.
nitpick: this is used exclusively in IngressUtil
, move this method to be private in the using class.
operator/controller/src/main/java/io/apicurio/registry/operator/util/IngressUtil.java
Show resolved
Hide resolved
operator/model/src/main/java/io/apicurio/registry/operator/api/v1/ApicurioRegistry3Spec.java
Show resolved
Hide resolved
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 almost ready, just missing a few tests accessing the instances through the Ingress
es.
Good job! 👍
Thank you! I'm working on the tests ATM. |
This comment was marked as outdated.
This comment was marked as outdated.
@andreaTP review please |
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.
A couple of nitpicks but LGTM 👍
No description provided.