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

Consume authorize_url from Secret #149

Closed
wants to merge 3 commits into from
Closed

Consume authorize_url from Secret #149

wants to merge 3 commits into from

Conversation

maorfr
Copy link
Contributor

@maorfr maorfr commented Jan 1, 2020

This PR intends to take the authorization URL out of the template and into the telemeter-server Secret.

Please let me know if you think this is a good idea!

Can you also point out where I'm missing a change in jsonnet?
Update: depends on openshift/telemeter#288

Thanks!

@metalmatze
Copy link
Contributor

The changes would need to be made in this file in the Telemeter repository:
https://github.com/openshift/telemeter/blob/master/jsonnet/telemeter/server/kubernetes.libsonnet
With the changes being merged over there we can then update the Telemeter dependency in this repository by running jb update.

@maorfr
Copy link
Contributor Author

maorfr commented Jan 20, 2020

wondering what i'm doing wrong @metalmatze

@metalmatze
Copy link
Contributor

Updating all jsonnet dependencies broke the current jsonnet stack.

@maorfr
Copy link
Contributor Author

maorfr commented Jan 27, 2020

@metalmatze care to take a look again?

@maorfr
Copy link
Contributor Author

maorfr commented Feb 4, 2020

/retest

@metalmatze
Copy link
Contributor

I am still not sure if we really want this in our templates. @squat what do you think?

@squat
Copy link
Member

squat commented Feb 4, 2020

We discussed with Maor a few weeks ago. It is not a change we like but, due to details/limitations of the saas-herder stack [1], it is required in order to enable deploying configmaps and other assets from outside of the app-interface repo. For that reason I think that the benefit outweighs the cost :/

[1] the tooling requires all https urls to be in a secret, as it incorrectly assumes that it is sensitive information.

@maorfr
Copy link
Contributor Author

maorfr commented Feb 11, 2020

these changes are now in master, closing this one.

EDIT: they are not, creating a replacement PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants