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.

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

[CLOUD-3200] - querying k8s api to discover generated routes #87

Merged
merged 1 commit into from May 9, 2019

Conversation

ricardozanini
Copy link
Contributor

@ricardozanini ricardozanini commented May 8, 2019

@luck3y this is the PR to fix https://issues.jboss.org/browse/CLOUD-3200.

Tested with Business Central (RHPAM) images and I was able to create a keycloak client configuration with URLs from the application routes where the pod is running.

The logic between this "discover" is to use the pod hostname. If a route is found with this name, we set the APPLICATION_ROUTES to it, then the script does the rest of the job and registers the keycloak client with the correct URLs.

Most of the time, templates create routes, services, pods, secrets with the same base name. So, if the template does not set the HOSTNAME_HTTP env variable, we try to found a URL based on this base name.

For this to work, I borrowed these functions from our code base (jboss-kie-modules):

  • query_route
  • query_route_host
  • build_simple_url
  • build_route_url

I tried to keep them agnostic to the use case, so we could easily export those to a common module in this repo. This would require the BA cloud team to add this new dependency to our images. That's why I'd like to discuss this matter with @errantepiphany and @spolti

So, before merging this one, I believe that we have to decide which path to go and/or if this feature make sense for you.

Signed-off-by: Ricardo Zanini zanini@redhat.com

luck3y
luck3y previously approved these changes May 8, 2019
os-eap-sso/added/keycloak.sh Outdated Show resolved Hide resolved
Copy link

@errantepiphany errantepiphany left a comment

Choose a reason for hiding this comment

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

I made a couple comments regarding the "secure-" prefix, and open to opinions on it.

os-eap-sso/added/keycloak.sh Outdated Show resolved Hide resolved
os-eap-sso/added/keycloak.sh Outdated Show resolved Hide resolved
@luck3y
Copy link
Collaborator

luck3y commented May 8, 2019

LGTM, thanks @ricardozanini

One final note, there aren't any tests, if you could write some bats tests just to verify the output of the functions, that would be really great.

os-eap-sso/added/keycloak.sh Outdated Show resolved Hide resolved
@ricardozanini
Copy link
Contributor Author

Guys,

I reviewed the entire scenario, reduced the scope and made a real clean up. Now we don't rely from kie-modules anymore (just a small error handling). The only "problem" now is the python dependency. Tomorrow I'll write bats tests and do the integration tests. @luck3y please hold this one until I finish these tests.

@errantepiphany @luck3y please let me know your thoughts. ❤️

@ricardozanini
Copy link
Contributor Author

ricardozanini commented May 8, 2019

Here's a sample output:

sh-4.2$ query_routes_from_service dasdas

sh-4.2$ query_routes_from_service eap-app
https://eap-app-bsig-cloud.192.168.99.100.nip.io
sh-4.2$ query_routes_from_service library-process-kieserver
http://library-process-kieserver-bsig-cloud.192.168.99.100.nip.io;https://secure-library-process-kieserver-bsig-cloud.192.168.99.100.nip.io

@wildfly-ci
Copy link
Collaborator

Build 348 outcome was UNKNOWN using a merge of e4da5a0
Summary: Canceled Build time: 00:16:52

@wildfly-ci
Copy link
Collaborator

Build 349 outcome was UNKNOWN using a merge of e4da5a0
Summary: Canceled Build time: 00:00:02

Signed-off-by: Ricardo Zanini <zanini@redhat.com>
@ricardozanini
Copy link
Contributor Author

ricardozanini commented May 9, 2019

Everything is fine now and this PR is ready to be merged after #86

@errantepiphany
Copy link

@ricardozanini , looks good to me, but will defer to @luck3y , as he is the gatekeeper of jboss-eap-modules.

@luck3y luck3y merged commit 9b80cf4 into jboss-container-images:master May 9, 2019
@ricardozanini ricardozanini deleted the cloud-3200 branch May 9, 2019 23:07
@wildfly-ci
Copy link
Collaborator

Build 358 outcome was UNKNOWN using a merge of a47004a
Summary: Canceled (Execution timeout (new)) Build time: 22:20:35

@luck3y
Copy link
Collaborator

luck3y commented May 10, 2019

Ignore the CI error, this was from a hung previous job.

luck3y added a commit to luck3y/jboss-eap-modules that referenced this pull request May 15, 2019
…-3200

[CLOUD-3200] - querying k8s api to discover generated routes
luck3y added a commit to luck3y/jboss-eap-modules that referenced this pull request May 15, 2019
…-3200

[CLOUD-3200] - querying k8s api to discover generated routes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants