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

Add helm options for config, pullPolicy and apiGroups #154

Merged
merged 2 commits into from
May 20, 2021

Conversation

tyrken
Copy link
Contributor

@tyrken tyrken commented May 20, 2021

I had to make some changes to the helm chart to get it usable in our cluster, including control over the bits needed from the external values file.

Note I had to define a non-empty displayContext also, to avoid getting stuck in a redirect loop, not sure if this a bug or not.

Sorry my auto-formatting editor has put in a bunch of whitespace fixups, hope you don't mind them included...

@salesforce-cla
Copy link

Thanks for the contribution! Before we can merge this, we need @tyrken to sign the Salesforce.com Contributor License Agreement.

@giovannirco
Copy link

I am glad you fixed the ingress indentation, I was about to push a PR for that but I also have had some other issues with the ingress, I wonder if it worked fine for you

@tyrken
Copy link
Contributor Author

tyrken commented May 20, 2021

Sorry didn't use the ingress (I proxy via https://github.com/buzzfeed/sso with it's own ingress) - but if your other issues were an infinite redirect I did get that until I added a non-empty displayContext.

Doing so in this PR is a bit of a fudge but not sure how best to "fix"

server.mux.HandleFunc("/", redirectHandler(config.CurrentContext))

@@ -4,10 +4,11 @@ replicas: 1
image:
tag: latest
repository: sloopimage/sloop
pullPolicy: Always
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we change it to IfNotPresent?

Copy link
Collaborator

@sana-jawad sana-jawad left a comment

Choose a reason for hiding this comment

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

Thanks a lot @tyrken for the contribution. Overall the change looks good. Just added a small comment.

@tyrken
Copy link
Contributor Author

tyrken commented May 20, 2021

Done @sana-jawad

@sana-jawad sana-jawad merged commit 1c49d1c into salesforce:master May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants