Update server-acl-init to always check for the deployed serviceAccountToken secret #1770
Conversation
kschoche
changed the title
| // In Kube 1.24+ there is no automatically generated long term JWT token for a ServiceAccount. | ||
| // Furthermore, there is no reference to a Secret in the ServiceAccount. Instead we have deployed | ||
| // a Secret in Helm which references the ServiceAccount and contains a permanent JWT token. | ||
| secretNames = append(secretNames, c.withPrefix("auth-method")) |
There was a problem hiding this comment.
I'm wondering about the order here for the Openshift case where it injects secrets into the ServiceAccount.
Are those supposed to hold the JWT token that we're supposed to use? if so I think we just need to re-order these four lines so that we attempt to read the Helm deployed Secret last?
kschoche
changed the title
kschoche
requested review from
a team,
analogue and
ishustava
and removed request for
a team
david-yu
requested review from
a team,
malizz and
erichaberkorn
and removed request for
analogue,
ishustava,
a team and
malizz
|
Have we been able to reproduce the associated issue and resolve it with this fix? |
|
Any movement on 1 approving review remaining? |
|
@rgish You should be able to make and run your own consul k8s images using the guide described here: https://github.com/hashicorp/consul-k8s/blob/main/CONTRIBUTING.md#contributing-101 |
Receive the following error in api-gateway-controller-acl-init:
But, server-acl-init-job does not display any errors in our OpenShift environment, so the commit should seem to be considered an improvement:
|
* Exclude openebs namespace from injection. OpenEBS is a Kubernetes storage solution. When you spin up a PVC, under the hood OpenEBS creates a pod to handle the necessary storage operations. If the openebs namespace is not excluded from injection, that pod can't start because our mutatingwebhook config requires all pod scheduling requests make it to our webhook and our webhook isn't running yet because the consul servers aren't running. This is a breaking change but I think it's worth it because it's very unlikely anyone is using the openebs namespace for anything other than openebs. * Changelog
The generate_lease=true configuration is unnecessary and generates a note about performance implications in Vault logs. Remove this configuration so that the default value of generate_lease=false is used instead.
* Remove gnupg * Update CHANGELOG.md
* Dockerfile: remove `gnupg` from dev image * Update CHANGELOG.md
- Add troubleshooting commands for 'upstream' and 'proxy' to allow troubleshooting of envoy config.
Move format parsing into envoy package Move enovy to common package, move param parsing to calling package use a LoggerParams struct for handling a format for log changes to envoy refactor to use logger params and methods to set and validate logger and log levels before calling envoy linting changes clean up from rebase Improve comment on envoy logging endpoint function, switched to using '-update-level' for updating envoy log level flag for better usability
* pinned consul test image to latest dev nightly - created a new variable for setting the consul test image used for the majority of tests - fixed some incorrect spacing * updated builtin lambda envoy extensions in tests
|
Looks like all test passed. @thisisnotashwin could you have another look? |
thisisnotashwin
force-pushed
the
test_fix_server_acl_init_openshift
branch
from
59af407 to
1940e96
Compare
Changes proposed in this PR:
SecretRefsintoServiceAccounts(one for service account and the other for docker registry credentials), even in Kubernetes 1.24+. In the current logic ofserver-acl-initwe expect to use the Secret deployed by the helm chart, but only in the case where the ServiceAccount does not contain SecretRefs. Due to OpenShift injecting these we never look for the deployed Secret for the consul-auth-method.How I've tested this PR:
Unit + acceptance tests should pass.
How I expect reviewers to test this PR:
👀
Looking for some review comments about the approach as well :)
Checklist: