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

feat: Allow users to disable extauth sidecar on a per proxy basis #8898

Merged
merged 6 commits into from
Nov 16, 2023

Conversation

davidjumani
Copy link
Contributor

@davidjumani davidjumani commented Nov 13, 2023

Description

When multiple gateway proxies are defined as well as extauth enabled, a user might want to enable the sidecar for a subset of gateway proxies (since some of them do not require extauth) to reduce resource usage and unnecessary network calls to an unused sidecar.
This feature adds the new helm value gatewayproxy.proxyName.disableExtauthSidecar to disable the extauth sidecar on it
It also adds additional helper methods to merge resources while omitting keys in those resources which should not be overwritten if empty

Helm changes

  • Added a new gatewayproxy.proxyName.disableExtauthSidecar value

Code changes

N/A

CI changes

N/A

Docs changes

N/A

Context

#8430

Interesting decisions

Had initially thought of the idea to omit fields in an overwrite based on keys as in ddc6774 but decided against it as future changes would require modifying those keys if the same field is modified in different locations. Eg: if one part of the code did not want the field modified it would add the key to the list of omitKeys but another part of the code when changing the field would need to remove it from the list

Testing steps

Unit tests in enterprise to validate the behaviour of the new helm fields and helpers

Notes for reviewers

N/A

Please proofread comments on ...

N/A

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

@github-actions github-actions bot added the keep pr updated signals bulldozer to keep pr up to date with base branch label Nov 13, 2023
@solo-changelog-bot
Copy link

Issues linked to changelog:
#8430

Copy link

github-actions bot commented Nov 14, 2023

Visit the preview URL for this PR (updated for commit e190aea):

https://gloo-edge--pr8898-disable-extauth-side-8ks7trcg.web.app

(expires Wed, 22 Nov 2023 04:23:26 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 77c2b86e287749579b7ff9cadb81e099042ef677

Copy link
Contributor

@sheidkamp sheidkamp left a comment

Choose a reason for hiding this comment

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

One nit, but LGTM

install/helm/gloo/generate/values.go Outdated Show resolved Hide resolved
@davidjumani davidjumani added the work in progress signals bulldozer to keep pr open (don't auto-merge) label Nov 14, 2023
@davidjumani davidjumani reopened this Nov 15, 2023
@davidjumani davidjumani removed the work in progress signals bulldozer to keep pr open (don't auto-merge) label Nov 16, 2023
@soloio-bulldozer soloio-bulldozer bot merged commit ce83291 into main Nov 16, 2023
26 checks passed
@soloio-bulldozer soloio-bulldozer bot deleted the disable-extauth-sidecar branch November 16, 2023 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep pr updated signals bulldozer to keep pr up to date with base branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants