Enable ingress gateways to use ACL Auth Method #1118
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jmurret
commented
Mar 23, 2022
| @@ -26,15 +26,15 @@ | |||
| apiVersion: apps/v1 | |||
| kind: Deployment | |||
| metadata: | |||
| name: {{ template "consul.fullname" $root }}-{{ .name }} | |||
| name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway | |||
There was a problem hiding this comment.
throughout this PR (and the terminating gateway one), we add -terminating-gateway to the end of all resources because unlike other components, users can launch multiple terminating gateways and multiple ingress gateways that they can name themselves.
One, we want to be able to identify the component type if the name it foo. Two, if they have an ingress gateway named foo and a terminating gateway named foo, this suffix will prevent name collisions / confusion.
There was a problem hiding this comment.
Not sure about this. We'll then have everything called consul-ingress-gateway-ingress-gateway
There was a problem hiding this comment.
I don't think most folks use multiple igw's so not sure we should optimize for that. What's the impact if they accidentally set both names to foo? Does it fail the helm install/upgrade?
There was a problem hiding this comment.
By default we have:
ingressGateways:
gateways:
- name: ingress-gatewayso all you need to do in your values is:
ingressGateways:
enabled: true
Now this would cause you to have consul-ingress-gateway-ingress-gateway everywhere which looks pretty bad imo.
There was a problem hiding this comment.
yea, I'm not sure of the result. I think we may wind up overwriting acl roles and policies where the last one wins and the losers get permission denied when they try to log in.
This resulted from this question on terminating gateways work: #1102 (comment)
I'll try out the scenario where we have 1 terminating gateway named the same as 1 ingress gateway with naming conventions that do not have this gateway type suffix. If there are issues, I'mhappy to change this and the terminating gateway stuff so its the right UX. Other options could:
- emphasis in documentation ingress gateways and terminating gateways need to be unique across all
- run a check in helm and fail
- both
There was a problem hiding this comment.
Looks like helm install fails before anything...as you inquired about. Getting errors related to duplicate service accounts. Getting serviceaccounts "test-nsy8zi-consul-foo" already exists.
@lkysow what do you think is the best option? I'm going to look into a helm check at the moment. I agree with you that this PR is over optimizing for a rare case. The check is probably going to involve checking:
- look for duplicate terminating gateways
- look for duplicate ingress gateways
- look for duplicates across all ingress and terminating gateway names.
I'm guessing each of these checks will involve nested loops. So, for us the code will be 🙈 , but I think it will be worth it for a better user UX. If there are other options or ideas, please let me know what you think.
There was a problem hiding this comment.
Yeah I think the helm check is a good idea and I apologize in advance for what is likely to be quite un-fun.
Can definitely be a separate PR at least.
There was a problem hiding this comment.
Cool. I address it in this PR: #1120
There was a problem hiding this comment.
That sounds good! Feels like a much better solution than repeated names
jmurret
commented
Mar 23, 2022
| {{- if $root.Values.global.adminPartitions.enabled }} | ||
| -partition={{ $root.Values.global.adminPartitions.name }} \ | ||
| {{- end }} | ||
| -token-sink-file=/consul/service/acl-token \ |
There was a problem hiding this comment.
save the acl token to this sink file location. Then we will use it later in setting the CONSUL_HTTP_TOKEN_FILE on ingress gateway container so that it can run with this ACL token.
jmurret
requested review from
ishustava,
a team and
ndhanushkodi
and removed request for
a team
Base automatically changed from
jm/terminating-gateway-token
to
acls-refactor-base-branch
March 23, 2022 23:45
jmurret
force-pushed
the
jm/ingress-gateway-token
branch
2 times, most recently
from
f5165cf to
6504918
Compare
ndhanushkodi
approved these changes
Mar 24, 2022
acceptance/tests/ingress-gateway/ingress_gateway_namespaces_test.go
Outdated
Show resolved
Hide resolved
| @@ -399,6 +404,9 @@ spec: | |||
| -partition={{ $root.Values.global.adminPartitions.name }} \ | |||
| {{- end }} | |||
| -id="${POD_NAME}" | |||
| {{- if $root.Values.global.acls.manageSystemACLs }} | |||
| - "/consul-bin/consul logout" | |||
| {{- end}} | |||
There was a problem hiding this comment.
If an ingress gateway is force shutdown and the consul logout isn't run, how are we cleaning up the token? Or did we decide it's ok not to?
There was a problem hiding this comment.
I am not really sure. If I had to guess how it would, I would think the next pod would log in again, which would destroy/expire the token in consul server and then the sink file would get updated with the new token. My guess is that a token (that is probably still valid) would still be in the sink file and usable, but the acl-init container of the next pod creation would ensure that login is the first thing that happens.
I'm not sure if this is correct in the way that it would work currently...and whether that is desired.
Thoughts @ishustava or @lkysow ?
There was a problem hiding this comment.
Also, whatever the answer is, this will be the case with all components that are now using consul login to get an acl token. They are all defining this preStop logout.
There was a problem hiding this comment.
Yeah we decided that we won't cleanup tokens in the case of a force-shutdown. The other option was to run a controller in k8s but it felt a bit heavy-handed for this and we thought we'll wait for user feedback before we implement it.
2 tasks
jmurret
force-pushed
the
acls-refactor-base-branch
branch
from
4230fe1 to
7d488de
Compare
* Refactor ConsulLogin() to return the acltoken in addition to theerror. * Refactor createACLPolicyRoleAndBindingRule toappend datacenters for local tokens. Refactor updateOrCreateBindingRule to create binding rule if there are binding rules but this one does not exist * Rename -create-client-token flag to -client * set additional sans for consul server load balancer so that client will be able to use the certificate to talk to the load balancers rather than just an individual server. * Refactor server-acl-init command to create ACL Policy and Rule for client component so that client can call ConsulLogin and receive and ACL Token Call. * Enable client to talk to Consul Server to perform consul login. * Pass Auth Method to k8s al-init command. * Configure Consul address to be the Consul Server Load Balancer. * Configure CA Cert volume to be in memory rather than k8s secret when using vault. * Set consul/login volume and CONSUL_HTTP_TOKEN_FILE for use during logout. * Setup prestop command to perform consul logout. * Configure client-daemonset so that we can utilize the externalServers setting to configure clients to be able to call consul login on a server that is on a different partition. * Configuring partition-init to remove additional flags and use ones that already exist * adding missing comma * fix flakey tests by wrapping asserts in retries a la Iryna * Adding -use-https flag to client-daemonset.yaml when externalServers are enabled * Refactoring tests to cover client-acl-init changes * addressing PR comments * removing mounted tmpfs for consul-ca-cert when using vault and restoring datacenter logic because of breaking test. * addressing PR comments and only appending datacenters to a policy when its a local token, not global tokens. * completing additional dns names based on PR feedback * Do not ca-cert volume when using vault. * removing unused flagConsulCACert from partition-init command * PR Feedback. Removing unused envvars in acl-init container. changing ConsulLogin to return secretID, error instead ok token, error.
…roles for terminating gateways.
…in the form of terminating-gateway/RELEASE-NAME-consul-terminating - <component type>/<consul fullname>-<name>
jmurret
force-pushed
the
jm/ingress-gateway-token
branch
from
51960fc to
b44c3e4
Compare
…st.go Co-authored-by: Nitya Dhanushkodi <[email protected]>
jmurret
force-pushed
the
jm/ingress-gateway-token
branch
from
b44c3e4 to
530b7d2
Compare
ishustava
approved these changes
Mar 30, 2022
| @@ -26,15 +26,15 @@ | |||
| apiVersion: apps/v1 | |||
| kind: Deployment | |||
| metadata: | |||
| name: {{ template "consul.fullname" $root }}-{{ .name }} | |||
| name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway | |||
There was a problem hiding this comment.
That sounds good! Feels like a much better solution than repeated names
| @@ -399,6 +404,9 @@ spec: | |||
| -partition={{ $root.Values.global.adminPartitions.name }} \ | |||
| {{- end }} | |||
| -id="${POD_NAME}" | |||
| {{- if $root.Values.global.acls.manageSystemACLs }} | |||
| - "/consul-bin/consul logout" | |||
| {{- end}} | |||
There was a problem hiding this comment.
Yeah we decided that we won't cleanup tokens in the case of a force-shutdown. The other option was to run a controller in k8s but it felt a bit heavy-handed for this and we thought we'll wait for user feedback before we implement it.
jmurret
added a commit
that referenced
this pull request
Mar 31, 2022
…e-configured bootstrap token as k8s secret. (#1128) * Use kube auth method to provision ACL token for the crd controller (#995) * Use a Consul Kubernetes Auth Method to issue consul-login to mint ACL tokens and consul-logout to clean them up for the CRD controller. Co-authored-by: Iryna Shustava <[email protected]> * Support storing bootstrap token in Vault (#1061) * Global auth method (#1075) • Update server-acl-init to create authmethods in the primary datacenter when the job is run in a secondary datacenter during federation. This authmethod allows us to issue logins for global policies. • Update the controller workflow in server-acl-init to use this global authmethod when run in a secondary DC. • Update the mesh-gateway acceptance tests to create proxy defaults in the secondary DC to test above behavior works successfully. • Updated logout to not pass in the partition flag as it is not required. • Update server acl init tests to migrate from require := require.New(t) to require.xyz(t, ...) patterns. * Refactor ConnectInject to use authmethods (#1076) Refactor connect-injector to use the new auth-method workflow when ACLs are enabled so that Kubernetes secrets are not used. * Sync token acl refactor (#1081) • Refactor sync-catalog to use the new auth-method workflow when ACLs are enabled so that Kubernetes secrets are not used. • Create a service account and rolebinding dedicated to the component authmethod so that it no longer piggybacks on the one used by the connect-inject authmethod. * rename the controller flag (#1089) * Refactor Consul API Gateway Controller to use AuthMethod workflow. (#1083) * Refactor Consul API Gateway Controller to use AuthMethod workflow. * Refactor snapshot agent to use new acl authmethod workflow (#1084) * refactor snapshot agent to use new acl authmethod workflow. * Refactor mesh-gateway ACL flow (#1085) * Refactor mesh-gateway ACL flow * Fix flakey server-acl-init tests with retries (#1095) * Fix flakey server-acl-init tests with retries * Adding retry for flakey server-acl-init enterprise test * adding missing retry module in server-acl-init enterprise tests * Update Binding Rule if it exists for the authmethod (#1094) * Enable ACL Client Token (#1093) * Refactor ConsulLogin() to return the acltoken in addition to theerror. * Refactor createACLPolicyRoleAndBindingRule toappend datacenters for local tokens. Refactor updateOrCreateBindingRule to create binding rule if there are binding rules but this one does not exist * Rename -create-client-token flag to -client * set additional sans for consul server load balancer so that client will be able to use the certificate to talk to the load balancers rather than just an individual server. * Refactor server-acl-init command to create ACL Policy and Rule for client component so that client can call ConsulLogin and receive and ACL Token Call. * Enable client to talk to Consul Server to perform consul login. * Pass Auth Method to k8s al-init command. * Configure Consul address to be the Consul Server Load Balancer. * Configure CA Cert volume to be in memory rather than k8s secret when using vault. * Set consul/login volume and CONSUL_HTTP_TOKEN_FILE for use during logout. * Setup prestop command to perform consul logout. * Configure client-daemonset so that we can utilize the externalServers setting to configure clients to be able to call consul login on a server that is on a different partition. * Configuring partition-init to remove additional flags and use ones that already exist * adding missing comma * fix flakey tests by wrapping asserts in retries a la Iryna * Adding -use-https flag to client-daemonset.yaml when externalServers are enabled * Refactoring tests to cover client-acl-init changes * addressing PR comments * removing mounted tmpfs for consul-ca-cert when using vault and restoring datacenter logic because of breaking test. * addressing PR comments and only appending datacenters to a policy when its a local token, not global tokens. * completing additional dns names based on PR feedback * Do not ca-cert volume when using vault. * removing unused flagConsulCACert from partition-init command * PR Feedback. Removing unused envvars in acl-init container. changing ConsulLogin to return secretID, error instead ok token, error. * vault: add support for admin partitions (#1098) * Refactor common.Login (#1101) * convert function args to a struct * add some missing tests * move logic that is only relevant for connect out * Use bootstrap token from vault to validate exec'ing into consul server (#1116) Follow up on #1103 * Enable terminating gateways to use ACL Auth Method (#1102) * Enable terminating gateway policy to be generated via Auth Method * Filtering out failing portion of test for terminating gateway work * PR feedback.Fixing tests. Changing naming conventions for policy and roles for terminating gateways. * Update control-plane/subcommand/server-acl-init/command.go Co-authored-by: Iryna Shustava <[email protected]> * Update control-plane/subcommand/server-acl-init/command.go Co-authored-by: Iryna Shustava <[email protected]> * Fixing enterprise tests * Changing terminating gateway to pass acl-init a -component-name flag in the form of terminating-gateway/RELEASE-NAME-consul-terminating - <component type>/<consul fullname>-<name> * fixing acceptance test to recognize that long lived tokens will not exist and we ahve to update the role. * Correcting serviceAccount used on deployment * Making all nameshavea-ingress-gateway * Update charts/consul/templates/terminating-gateways-deployment.yaml Co-authored-by: Iryna Shustava <[email protected]> * Update control-plane/subcommand/server-acl-init/command.go Co-authored-by: Iryna Shustava <[email protected]> * Update control-plane/subcommand/server-acl-init/command.go Co-authored-by: Iryna Shustava <[email protected]> * Update control-plane/subcommand/server-acl-init/command.go Co-authored-by: Iryna Shustava <[email protected]> Co-authored-by: Iryna Shustava <[email protected]> * Enable snapshot agent configuration to be retrieved from vault (#1113) * Enable ACL Client Token (#1093) * Refactor ConsulLogin() to return the acltoken in addition to theerror. * Refactor createACLPolicyRoleAndBindingRule toappend datacenters for local tokens. Refactor updateOrCreateBindingRule to create binding rule if there are binding rules but this one does not exist * Rename -create-client-token flag to -client * set additional sans for consul server load balancer so that client will be able to use the certificate to talk to the load balancers rather than just an individual server. * Refactor server-acl-init command to create ACL Policy and Rule for client component so that client can call ConsulLogin and receive and ACL Token Call. * Enable client to talk to Consul Server to perform consul login. * Pass Auth Method to k8s al-init command. * Configure Consul address to be the Consul Server Load Balancer. * Configure CA Cert volume to be in memory rather than k8s secret when using vault. * Set consul/login volume and CONSUL_HTTP_TOKEN_FILE for use during logout. * Setup prestop command to perform consul logout. * Configure client-daemonset so that we can utilize the externalServers setting to configure clients to be able to call consul login on a server that is on a different partition. * Configuring partition-init to remove additional flags and use ones that already exist * adding missing comma * fix flakey tests by wrapping asserts in retries a la Iryna * Adding -use-https flag to client-daemonset.yaml when externalServers are enabled * Refactoring tests to cover client-acl-init changes * addressing PR comments * removing mounted tmpfs for consul-ca-cert when using vault and restoring datacenter logic because of breaking test. * addressing PR comments and only appending datacenters to a policy when its a local token, not global tokens. * completing additional dns names based on PR feedback * Do not ca-cert volume when using vault. * removing unused flagConsulCACert from partition-init command * PR Feedback. Removing unused envvars in acl-init container. changing ConsulLogin to return secretID, error instead ok token, error. * Enable snapshot agent configuration to be retrieved from vault * Adding CHANGELOG entry * Changing the decoding of snapshot agent config in vault to platform agnostic * Fixing the way we write the encoded vault secret out to a decoded json file * Decoding vault secret using consul template function on the vault annotation. Able to remove the bash that decodes the file and changes the extension. * Update CHANGELOG.md Co-authored-by: Iryna Shustava <[email protected]> * Update charts/consul/values.yaml Co-authored-by: Iryna Shustava <[email protected]> * Update charts/consul/values.yaml Co-authored-by: Iryna Shustava <[email protected]> * Update charts/consul/values.yaml Co-authored-by: Iryna Shustava <[email protected]> * PR Feedback - change client-snapshot-deployment to only have one vault role entry even when needing to set to vault roles * PR Feedback - when both snapshot agent and ca roles are specified in vault, it should get the sa role. * Simplifying conditional for vault role. Co-authored-by: Iryna Shustava <[email protected]> * Ability to set initial_management token when using k8s secret store. Snapshot agent acceptance tests (#1125) * Enable ACL Client Token (#1093) * Refactor ConsulLogin() to return the acltoken in addition to theerror. * Refactor createACLPolicyRoleAndBindingRule toappend datacenters for local tokens. Refactor updateOrCreateBindingRule to create binding rule if there are binding rules but this one does not exist * Rename -create-client-token flag to -client * set additional sans for consul server load balancer so that client will be able to use the certificate to talk to the load balancers rather than just an individual server. * Refactor server-acl-init command to create ACL Policy and Rule for client component so that client can call ConsulLogin and receive and ACL Token Call. * Enable client to talk to Consul Server to perform consul login. * Pass Auth Method to k8s al-init command. * Configure Consul address to be the Consul Server Load Balancer. * Configure CA Cert volume to be in memory rather than k8s secret when using vault. * Set consul/login volume and CONSUL_HTTP_TOKEN_FILE for use during logout. * Setup prestop command to perform consul logout. * Configure client-daemonset so that we can utilize the externalServers setting to configure clients to be able to call consul login on a server that is on a different partition. * Configuring partition-init to remove additional flags and use ones that already exist * adding missing comma * fix flakey tests by wrapping asserts in retries a la Iryna * Adding -use-https flag to client-daemonset.yaml when externalServers are enabled * Refactoring tests to cover client-acl-init changes * addressing PR comments * removing mounted tmpfs for consul-ca-cert when using vault and restoring datacenter logic because of breaking test. * addressing PR comments and only appending datacenters to a policy when its a local token, not global tokens. * completing additional dns names based on PR feedback * Do not ca-cert volume when using vault. * removing unused flagConsulCACert from partition-init command * PR Feedback. Removing unused envvars in acl-init container. changing ConsulLogin to return secretID, error instead ok token, error. * Enable snapshot agent configuration to be retrieved from vault * Adding CHANGELOG entry * Changing the decoding of snapshot agent config in vault to platform agnostic * Fixing the way we write the encoded vault secret out to a decoded json file * Decoding vault secret using consul template function on the vault annotation. Able to remove the bash that decodes the file and changes the extension. * Adding an acceptance test for snapshot agent. It currently fails because of a bug with Consul where it does not recognize CONSUL_HTTP_TOKEN. Will need to refactor test to bootstrap, then create vault secret with embedded acl token, then helm upgrade to add snapshot agent. Then assert that a *.snap file is created. * Adding acceptance test for snapshot agent on vault. * renaming test and removing extra file * Move vault test helpers into framework folder so we can use it more easily from other folders. * Adding snapshot agent test for k8s secret * Adding ability to set initial_management token when using k8s secrets. Also working acceptance test for snapshot agent on k8s secrets. * Adding bats tests. Adding envvar for ACL_BOOTSTRAP_TOKEN. Removing volume and volume mounts for bootstrap token. * Adding CHANGELOG entry for ability to pre-set bootstrap ACL token * Fixing bats tests * Update acceptance/framework/consul/helm_cluster.go Co-authored-by: Thomas Eckert <[email protected]> * Fixing broken unit tests * Lowering snapshot interval from 1mto15s for tests * Update acceptance/framework/consul/helm_cluster.go Co-authored-by: Nitya Dhanushkodi <[email protected]> * Update acceptance/framework/vault/helpers.go Co-authored-by: Nitya Dhanushkodi <[email protected]> * Update acceptance/tests/snapshot-agent/snapshot_agent_vault_test.go Co-authored-by: Nitya Dhanushkodi <[email protected]> * PR Feedback - clarify comments on Vault helper functions * PR Feedback - clarify comments on Vault helper functions * Modifying tests to not incidentally send an encoded file * Removing logging token in acceptance test code. Co-authored-by: Thomas Eckert <[email protected]> Co-authored-by: Nitya Dhanushkodi <[email protected]> * Enable ingress gateways to use ACL Auth Method (#1118) * Enable ACL Client Token (#1093) * Refactor ConsulLogin() to return the acltoken in addition to theerror. * Refactor createACLPolicyRoleAndBindingRule toappend datacenters for local tokens. Refactor updateOrCreateBindingRule to create binding rule if there are binding rules but this one does not exist * Rename -create-client-token flag to -client * set additional sans for consul server load balancer so that client will be able to use the certificate to talk to the load balancers rather than just an individual server. * Refactor server-acl-init command to create ACL Policy and Rule for client component so that client can call ConsulLogin and receive and ACL Token Call. * Enable client to talk to Consul Server to perform consul login. * Pass Auth Method to k8s al-init command. * Configure Consul address to be the Consul Server Load Balancer. * Configure CA Cert volume to be in memory rather than k8s secret when using vault. * Set consul/login volume and CONSUL_HTTP_TOKEN_FILE for use during logout. * Setup prestop command to perform consul logout. * Configure client-daemonset so that we can utilize the externalServers setting to configure clients to be able to call consul login on a server that is on a different partition. * Configuring partition-init to remove additional flags and use ones that already exist * adding missing comma * fix flakey tests by wrapping asserts in retries a la Iryna * Adding -use-https flag to client-daemonset.yaml when externalServers are enabled * Refactoring tests to cover client-acl-init changes * addressing PR comments * removing mounted tmpfs for consul-ca-cert when using vault and restoring datacenter logic because of breaking test. * addressing PR comments and only appending datacenters to a policy when its a local token, not global tokens. * completing additional dns names based on PR feedback * Do not ca-cert volume when using vault. * removing unused flagConsulCACert from partition-init command * PR Feedback. Removing unused envvars in acl-init container. changing ConsulLogin to return secretID, error instead ok token, error. * Enable terminating gateway policy to be generated via Auth Method * Filtering out failing portion of test for terminating gateway work * PR feedback.Fixing tests. Changing naming conventions for policy and roles for terminating gateways. * Changing terminating gateway to pass acl-init a -component-name flag in the form of terminating-gateway/RELEASE-NAME-consul-terminating - <component type>/<consul fullname>-<name> * Correcting serviceAccount used on deployment * Making all nameshavea-ingress-gateway * Enable ingress gateway policy to be generated via Auth Method * Making all names have a -ingress-gateway suffix * Removing duplicate test * Update acceptance/tests/ingress-gateway/ingress_gateway_namespaces_test.go Co-authored-by: Nitya Dhanushkodi <[email protected]> Co-authored-by: Nitya Dhanushkodi <[email protected]> * Removing the gateway type suffix from the naming conventions for terminating and ingress gateways (#1120) * Enable ACL Client Token (#1093) * Refactor ConsulLogin() to return the acltoken in addition to theerror. * Refactor createACLPolicyRoleAndBindingRule toappend datacenters for local tokens. Refactor updateOrCreateBindingRule to create binding rule if there are binding rules but this one does not exist * Rename -create-client-token flag to -client * set additional sans for consul server load balancer so that client will be able to use the certificate to talk to the load balancers rather than just an individual server. * Refactor server-acl-init command to create ACL Policy and Rule for client component so that client can call ConsulLogin and receive and ACL Token Call. * Enable client to talk to Consul Server to perform consul login. * Pass Auth Method to k8s al-init command. * Configure Consul address to be the Consul Server Load Balancer. * Configure CA Cert volume to be in memory rather than k8s secret when using vault. * Set consul/login volume and CONSUL_HTTP_TOKEN_FILE for use during logout. * Setup prestop command to perform consul logout. * Configure client-daemonset so that we can utilize the externalServers setting to configure clients to be able to call consul login on a server that is on a different partition. * Configuring partition-init to remove additional flags and use ones that already exist * adding missing comma * fix flakey tests by wrapping asserts in retries a la Iryna * Adding -use-https flag to client-daemonset.yaml when externalServers are enabled * Refactoring tests to cover client-acl-init changes * addressing PR comments * removing mounted tmpfs for consul-ca-cert when using vault and restoring datacenter logic because of breaking test. * addressing PR comments and only appending datacenters to a policy when its a local token, not global tokens. * completing additional dns names based on PR feedback * Do not ca-cert volume when using vault. * removing unused flagConsulCACert from partition-init command * PR Feedback. Removing unused envvars in acl-init container. changing ConsulLogin to return secretID, error instead ok token, error. * Enable terminating gateway policy to be generated via Auth Method * Filtering out failing portion of test for terminating gateway work * PR feedback.Fixing tests. Changing naming conventions for policy and roles for terminating gateways. * Correcting serviceAccount used on deployment * Making all nameshavea-ingress-gateway * Enable ingress gateway policy to be generated via Auth Method * Making all names have a -ingress-gateway suffix * Removing duplicate test * Update acceptance/tests/ingress-gateway/ingress_gateway_namespaces_test.go Co-authored-by: Nitya Dhanushkodi <[email protected]> * Removing the gateway type suffix from the naming conventions for terminating and ingress gateways * Adding check for duplicate terminating gateways and ingress gateway names * Update charts/consul/templates/ingress-gateways-deployment.yaml Co-authored-by: Luke Kysow <[email protected]> * PR Feedback - adding the duplicate name found to the check failures for duplicate ingress or terminating gateway names * Fixing rebase conflict * Merge Conflict- duplicate test * Adding a 10s sleep to flakey snapshot agent tests that were not finding a snapshot in time. Co-authored-by: Nitya Dhanushkodi <[email protected]> Co-authored-by: Luke Kysow <[email protected]> Co-authored-by: Kyle Schochenmaier <[email protected]> Co-authored-by: Iryna Shustava <[email protected]> Co-authored-by: Ashwin Venkatesh <[email protected]> Co-authored-by: Thomas Eckert <[email protected]> Co-authored-by: Nitya Dhanushkodi <[email protected]> Co-authored-by: Luke Kysow <[email protected]>
jmurret
added a commit
that referenced
this pull request
Apr 4, 2022
* Use kube auth method to provision ACL token for the crd controller (#995) * Use a Consul Kubernetes Auth Method to issue consul-login to mint ACL tokens and consul-logout to clean them up for the CRD controller. Co-authored-by: Iryna Shustava <[email protected]> * Support storing bootstrap token in Vault (#1061) * Global auth method (#1075) • Update server-acl-init to create authmethods in the primary datacenter when the job is run in a secondary datacenter during federation. This authmethod allows us to issue logins for global policies. • Update the controller workflow in server-acl-init to use this global authmethod when run in a secondary DC. • Update the mesh-gateway acceptance tests to create proxy defaults in the secondary DC to test above behavior works successfully. • Updated logout to not pass in the partition flag as it is not required. • Update server acl init tests to migrate from require := require.New(t) to require.xyz(t, ...) patterns. * Refactor ConnectInject to use authmethods (#1076) Refactor connect-injector to use the new auth-method workflow when ACLs are enabled so that Kubernetes secrets are not used. * Sync token acl refactor (#1081) • Refactor sync-catalog to use the new auth-method workflow when ACLs are enabled so that Kubernetes secrets are not used. • Create a service account and rolebinding dedicated to the component authmethod so that it no longer piggybacks on the one used by the connect-inject authmethod. * rename the controller flag (#1089) * Refactor Consul API Gateway Controller to use AuthMethod workflow. (#1083) * Refactor Consul API Gateway Controller to use AuthMethod workflow. * Refactor snapshot agent to use new acl authmethod workflow (#1084) * refactor snapshot agent to use new acl authmethod workflow. * Refactor mesh-gateway ACL flow (#1085) * Refactor mesh-gateway ACL flow * Fix flakey server-acl-init tests with retries (#1095) * Fix flakey server-acl-init tests with retries * Adding retry for flakey server-acl-init enterprise test * adding missing retry module in server-acl-init enterprise tests * Update Binding Rule if it exists for the authmethod (#1094) * Enable ACL Client Token (#1093) * Refactor ConsulLogin() to return the acltoken in addition to theerror. * Refactor createACLPolicyRoleAndBindingRule toappend datacenters for local tokens. Refactor updateOrCreateBindingRule to create binding rule if there are binding rules but this one does not exist * Rename -create-client-token flag to -client * set additional sans for consul server load balancer so that client will be able to use the certificate to talk to the load balancers rather than just an individual server. * Refactor server-acl-init command to create ACL Policy and Rule for client component so that client can call ConsulLogin and receive and ACL Token Call. * Enable client to talk to Consul Server to perform consul login. * Pass Auth Method to k8s al-init command. * Configure Consul address to be the Consul Server Load Balancer. * Configure CA Cert volume to be in memory rather than k8s secret when using vault. * Set consul/login volume and CONSUL_HTTP_TOKEN_FILE for use during logout. * Setup prestop command to perform consul logout. * Configure client-daemonset so that we can utilize the externalServers setting to configure clients to be able to call consul login on a server that is on a different partition. * Configuring partition-init to remove additional flags and use ones that already exist * adding missing comma * fix flakey tests by wrapping asserts in retries a la Iryna * Adding -use-https flag to client-daemonset.yaml when externalServers are enabled * Refactoring tests to cover client-acl-init changes * addressing PR comments * removing mounted tmpfs for consul-ca-cert when using vault and restoring datacenter logic because of breaking test. * addressing PR comments and only appending datacenters to a policy when its a local token, not global tokens. * completing additional dns names based on PR feedback * Do not ca-cert volume when using vault. * removing unused flagConsulCACert from partition-init command * PR Feedback. Removing unused envvars in acl-init container. changing ConsulLogin to return secretID, error instead ok token, error. * vault: add support for admin partitions (#1098) * Refactor common.Login (#1101) * convert function args to a struct * add some missing tests * move logic that is only relevant for connect out * Use bootstrap token from vault to validate exec'ing into consul server (#1116) Follow up on #1103 * Enable terminating gateways to use ACL Auth Method (#1102) * Enable terminating gateway policy to be generated via Auth Method * Filtering out failing portion of test for terminating gateway work * PR feedback.Fixing tests. Changing naming conventions for policy and roles for terminating gateways. * Update control-plane/subcommand/server-acl-init/command.go Co-authored-by: Iryna Shustava <[email protected]> * Update control-plane/subcommand/server-acl-init/command.go Co-authored-by: Iryna Shustava <[email protected]> * Fixing enterprise tests * Changing terminating gateway to pass acl-init a -component-name flag in the form of terminating-gateway/RELEASE-NAME-consul-terminating - <component type>/<consul fullname>-<name> * fixing acceptance test to recognize that long lived tokens will not exist and we ahve to update the role. * Correcting serviceAccount used on deployment * Making all nameshavea-ingress-gateway * Update charts/consul/templates/terminating-gateways-deployment.yaml Co-authored-by: Iryna Shustava <[email protected]> * Update control-plane/subcommand/server-acl-init/command.go Co-authored-by: Iryna Shustava <[email protected]> * Update control-plane/subcommand/server-acl-init/command.go Co-authored-by: Iryna Shustava <[email protected]> * Update control-plane/subcommand/server-acl-init/command.go Co-authored-by: Iryna Shustava <[email protected]> Co-authored-by: Iryna Shustava <[email protected]> * Enable snapshot agent configuration to be retrieved from vault (#1113) * Enable ACL Client Token (#1093) * Refactor ConsulLogin() to return the acltoken in addition to theerror. * Refactor createACLPolicyRoleAndBindingRule toappend datacenters for local tokens. Refactor updateOrCreateBindingRule to create binding rule if there are binding rules but this one does not exist * Rename -create-client-token flag to -client * set additional sans for consul server load balancer so that client will be able to use the certificate to talk to the load balancers rather than just an individual server. * Refactor server-acl-init command to create ACL Policy and Rule for client component so that client can call ConsulLogin and receive and ACL Token Call. * Enable client to talk to Consul Server to perform consul login. * Pass Auth Method to k8s al-init command. * Configure Consul address to be the Consul Server Load Balancer. * Configure CA Cert volume to be in memory rather than k8s secret when using vault. * Set consul/login volume and CONSUL_HTTP_TOKEN_FILE for use during logout. * Setup prestop command to perform consul logout. * Configure client-daemonset so that we can utilize the externalServers setting to configure clients to be able to call consul login on a server that is on a different partition. * Configuring partition-init to remove additional flags and use ones that already exist * adding missing comma * fix flakey tests by wrapping asserts in retries a la Iryna * Adding -use-https flag to client-daemonset.yaml when externalServers are enabled * Refactoring tests to cover client-acl-init changes * addressing PR comments * removing mounted tmpfs for consul-ca-cert when using vault and restoring datacenter logic because of breaking test. * addressing PR comments and only appending datacenters to a policy when its a local token, not global tokens. * completing additional dns names based on PR feedback * Do not ca-cert volume when using vault. * removing unused flagConsulCACert from partition-init command * PR Feedback. Removing unused envvars in acl-init container. changing ConsulLogin to return secretID, error instead ok token, error. * Enable snapshot agent configuration to be retrieved from vault * Adding CHANGELOG entry * Changing the decoding of snapshot agent config in vault to platform agnostic * Fixing the way we write the encoded vault secret out to a decoded json file * Decoding vault secret using consul template function on the vault annotation. Able to remove the bash that decodes the file and changes the extension. * Update CHANGELOG.md Co-authored-by: Iryna Shustava <[email protected]> * Update charts/consul/values.yaml Co-authored-by: Iryna Shustava <[email protected]> * Update charts/consul/values.yaml Co-authored-by: Iryna Shustava <[email protected]> * Update charts/consul/values.yaml Co-authored-by: Iryna Shustava <[email protected]> * PR Feedback - change client-snapshot-deployment to only have one vault role entry even when needing to set to vault roles * PR Feedback - when both snapshot agent and ca roles are specified in vault, it should get the sa role. * Simplifying conditional for vault role. Co-authored-by: Iryna Shustava <[email protected]> * Ability to set initial_management token when using k8s secret store. Snapshot agent acceptance tests (#1125) * Enable ACL Client Token (#1093) * Refactor ConsulLogin() to return the acltoken in addition to theerror. * Refactor createACLPolicyRoleAndBindingRule toappend datacenters for local tokens. Refactor updateOrCreateBindingRule to create binding rule if there are binding rules but this one does not exist * Rename -create-client-token flag to -client * set additional sans for consul server load balancer so that client will be able to use the certificate to talk to the load balancers rather than just an individual server. * Refactor server-acl-init command to create ACL Policy and Rule for client component so that client can call ConsulLogin and receive and ACL Token Call. * Enable client to talk to Consul Server to perform consul login. * Pass Auth Method to k8s al-init command. * Configure Consul address to be the Consul Server Load Balancer. * Configure CA Cert volume to be in memory rather than k8s secret when using vault. * Set consul/login volume and CONSUL_HTTP_TOKEN_FILE for use during logout. * Setup prestop command to perform consul logout. * Configure client-daemonset so that we can utilize the externalServers setting to configure clients to be able to call consul login on a server that is on a different partition. * Configuring partition-init to remove additional flags and use ones that already exist * adding missing comma * fix flakey tests by wrapping asserts in retries a la Iryna * Adding -use-https flag to client-daemonset.yaml when externalServers are enabled * Refactoring tests to cover client-acl-init changes * addressing PR comments * removing mounted tmpfs for consul-ca-cert when using vault and restoring datacenter logic because of breaking test. * addressing PR comments and only appending datacenters to a policy when its a local token, not global tokens. * completing additional dns names based on PR feedback * Do not ca-cert volume when using vault. * removing unused flagConsulCACert from partition-init command * PR Feedback. Removing unused envvars in acl-init container. changing ConsulLogin to return secretID, error instead ok token, error. * Enable snapshot agent configuration to be retrieved from vault * Adding CHANGELOG entry * Changing the decoding of snapshot agent config in vault to platform agnostic * Fixing the way we write the encoded vault secret out to a decoded json file * Decoding vault secret using consul template function on the vault annotation. Able to remove the bash that decodes the file and changes the extension. * Adding an acceptance test for snapshot agent. It currently fails because of a bug with Consul where it does not recognize CONSUL_HTTP_TOKEN. Will need to refactor test to bootstrap, then create vault secret with embedded acl token, then helm upgrade to add snapshot agent. Then assert that a *.snap file is created. * Adding acceptance test for snapshot agent on vault. * renaming test and removing extra file * Move vault test helpers into framework folder so we can use it more easily from other folders. * Adding snapshot agent test for k8s secret * Adding ability to set initial_management token when using k8s secrets. Also working acceptance test for snapshot agent on k8s secrets. * Adding bats tests. Adding envvar for ACL_BOOTSTRAP_TOKEN. Removing volume and volume mounts for bootstrap token. * Adding CHANGELOG entry for ability to pre-set bootstrap ACL token * Fixing bats tests * Update acceptance/framework/consul/helm_cluster.go Co-authored-by: Thomas Eckert <[email protected]> * Fixing broken unit tests * Lowering snapshot interval from 1mto15s for tests * Update acceptance/framework/consul/helm_cluster.go Co-authored-by: Nitya Dhanushkodi <[email protected]> * Update acceptance/framework/vault/helpers.go Co-authored-by: Nitya Dhanushkodi <[email protected]> * Update acceptance/tests/snapshot-agent/snapshot_agent_vault_test.go Co-authored-by: Nitya Dhanushkodi <[email protected]> * PR Feedback - clarify comments on Vault helper functions * PR Feedback - clarify comments on Vault helper functions * Modifying tests to not incidentally send an encoded file * Removing logging token in acceptance test code. Co-authored-by: Thomas Eckert <[email protected]> Co-authored-by: Nitya Dhanushkodi <[email protected]> * Enable ingress gateways to use ACL Auth Method (#1118) * Enable ACL Client Token (#1093) * Refactor ConsulLogin() to return the acltoken in addition to theerror. * Refactor createACLPolicyRoleAndBindingRule toappend datacenters for local tokens. Refactor updateOrCreateBindingRule to create binding rule if there are binding rules but this one does not exist * Rename -create-client-token flag to -client * set additional sans for consul server load balancer so that client will be able to use the certificate to talk to the load balancers rather than just an individual server. * Refactor server-acl-init command to create ACL Policy and Rule for client component so that client can call ConsulLogin and receive and ACL Token Call. * Enable client to talk to Consul Server to perform consul login. * Pass Auth Method to k8s al-init command. * Configure Consul address to be the Consul Server Load Balancer. * Configure CA Cert volume to be in memory rather than k8s secret when using vault. * Set consul/login volume and CONSUL_HTTP_TOKEN_FILE for use during logout. * Setup prestop command to perform consul logout. * Configure client-daemonset so that we can utilize the externalServers setting to configure clients to be able to call consul login on a server that is on a different partition. * Configuring partition-init to remove additional flags and use ones that already exist * adding missing comma * fix flakey tests by wrapping asserts in retries a la Iryna * Adding -use-https flag to client-daemonset.yaml when externalServers are enabled * Refactoring tests to cover client-acl-init changes * addressing PR comments * removing mounted tmpfs for consul-ca-cert when using vault and restoring datacenter logic because of breaking test. * addressing PR comments and only appending datacenters to a policy when its a local token, not global tokens. * completing additional dns names based on PR feedback * Do not ca-cert volume when using vault. * removing unused flagConsulCACert from partition-init command * PR Feedback. Removing unused envvars in acl-init container. changing ConsulLogin to return secretID, error instead ok token, error. * Enable terminating gateway policy to be generated via Auth Method * Filtering out failing portion of test for terminating gateway work * PR feedback.Fixing tests. Changing naming conventions for policy and roles for terminating gateways. * Changing terminating gateway to pass acl-init a -component-name flag in the form of terminating-gateway/RELEASE-NAME-consul-terminating - <component type>/<consul fullname>-<name> * Correcting serviceAccount used on deployment * Making all nameshavea-ingress-gateway * Enable ingress gateway policy to be generated via Auth Method * Making all names have a -ingress-gateway suffix * Removing duplicate test * Update acceptance/tests/ingress-gateway/ingress_gateway_namespaces_test.go Co-authored-by: Nitya Dhanushkodi <[email protected]> Co-authored-by: Nitya Dhanushkodi <[email protected]> * Removing the gateway type suffix from the naming conventions for terminating and ingress gateways (#1120) * Enable ACL Client Token (#1093) * Refactor ConsulLogin() to return the acltoken in addition to theerror. * Refactor createACLPolicyRoleAndBindingRule toappend datacenters for local tokens. Refactor updateOrCreateBindingRule to create binding rule if there are binding rules but this one does not exist * Rename -create-client-token flag to -client * set additional sans for consul server load balancer so that client will be able to use the certificate to talk to the load balancers rather than just an individual server. * Refactor server-acl-init command to create ACL Policy and Rule for client component so that client can call ConsulLogin and receive and ACL Token Call. * Enable client to talk to Consul Server to perform consul login. * Pass Auth Method to k8s al-init command. * Configure Consul address to be the Consul Server Load Balancer. * Configure CA Cert volume to be in memory rather than k8s secret when using vault. * Set consul/login volume and CONSUL_HTTP_TOKEN_FILE for use during logout. * Setup prestop command to perform consul logout. * Configure client-daemonset so that we can utilize the externalServers setting to configure clients to be able to call consul login on a server that is on a different partition. * Configuring partition-init to remove additional flags and use ones that already exist * adding missing comma * fix flakey tests by wrapping asserts in retries a la Iryna * Adding -use-https flag to client-daemonset.yaml when externalServers are enabled * Refactoring tests to cover client-acl-init changes * addressing PR comments * removing mounted tmpfs for consul-ca-cert when using vault and restoring datacenter logic because of breaking test. * addressing PR comments and only appending datacenters to a policy when its a local token, not global tokens. * completing additional dns names based on PR feedback * Do not ca-cert volume when using vault. * removing unused flagConsulCACert from partition-init command * PR Feedback. Removing unused envvars in acl-init container. changing ConsulLogin to return secretID, error instead ok token, error. * Enable terminating gateway policy to be generated via Auth Method * Filtering out failing portion of test for terminating gateway work * PR feedback.Fixing tests. Changing naming conventions for policy and roles for terminating gateways. * Correcting serviceAccount used on deployment * Making all nameshavea-ingress-gateway * Enable ingress gateway policy to be generated via Auth Method * Making all names have a -ingress-gateway suffix * Removing duplicate test * Update acceptance/tests/ingress-gateway/ingress_gateway_namespaces_test.go Co-authored-by: Nitya Dhanushkodi <[email protected]> * Removing the gateway type suffix from the naming conventions for terminating and ingress gateways * Adding check for duplicate terminating gateways and ingress gateway names * Update charts/consul/templates/ingress-gateways-deployment.yaml Co-authored-by: Luke Kysow <[email protected]> * PR Feedback - adding the duplicate name found to the check failures for duplicate ingress or terminating gateway names * Fixing rebase conflict * Merge Conflict- duplicate test * Adding a 10s sleep to flakey snapshot agent tests that were not finding a snapshot in time. Co-authored-by: Nitya Dhanushkodi <[email protected]> Co-authored-by: Luke Kysow <[email protected]> * Enable ACL Client Token (#1093) * Refactor ConsulLogin() to return the acltoken in addition to theerror. * Refactor createACLPolicyRoleAndBindingRule toappend datacenters for local tokens. Refactor updateOrCreateBindingRule to create binding rule if there are binding rules but this one does not exist * Rename -create-client-token flag to -client * set additional sans for consul server load balancer so that client will be able to use the certificate to talk to the load balancers rather than just an individual server. * Refactor server-acl-init command to create ACL Policy and Rule for client component so that client can call ConsulLogin and receive and ACL Token Call. * Enable client to talk to Consul Server to perform consul login. * Pass Auth Method to k8s al-init command. * Configure Consul address to be the Consul Server Load Balancer. * Configure CA Cert volume to be in memory rather than k8s secret when using vault. * Set consul/login volume and CONSUL_HTTP_TOKEN_FILE for use during logout. * Setup prestop command to perform consul logout. * Configure client-daemonset so that we can utilize the externalServers setting to configure clients to be able to call consul login on a server that is on a different partition. * Configuring partition-init to remove additional flags and use ones that already exist * adding missing comma * fix flakey tests by wrapping asserts in retries a la Iryna * Adding -use-https flag to client-daemonset.yaml when externalServers are enabled * Refactoring tests to cover client-acl-init changes * addressing PR comments * removing mounted tmpfs for consul-ca-cert when using vault and restoring datacenter logic because of breaking test. * addressing PR comments and only appending datacenters to a policy when its a local token, not global tokens. * completing additional dns names based on PR feedback * Do not ca-cert volume when using vault. * removing unused flagConsulCACert from partition-init command * PR Feedback. Removing unused envvars in acl-init container. changing ConsulLogin to return secretID, error instead ok token, error. * Updating changelog for ACLs work. * Update CHANGELOG.md Co-authored-by: Iryna Shustava <[email protected]> * Update CHANGELOG.md Co-authored-by: Iryna Shustava <[email protected]> * Update CHANGELOG.md Co-authored-by: Iryna Shustava <[email protected]> * Adding note about old acl token cleanup. Adding note about configuring k8sAuthMethodHost in secondary datacenters when using mesh gateways in mesh federation. * Update CHANGELOG.md Co-authored-by: Iryna Shustava <[email protected]> Co-authored-by: Kyle Schochenmaier <[email protected]> Co-authored-by: Iryna Shustava <[email protected]> Co-authored-by: Ashwin Venkatesh <[email protected]> Co-authored-by: Thomas Eckert <[email protected]> Co-authored-by: Nitya Dhanushkodi <[email protected]> Co-authored-by: Luke Kysow <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes proposed in this PR:
acl-auth-methodso it can properly call Consul login.How I've tested this PR:
How I expect reviewers to test this PR:
👀
Checklist: