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.

Sign up for GitHub

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

Backport of Update server-acl-init to always check for the deployed serviceAccountToken secret into release/1.0.x #1905

Closed
wants to merge 2 commits into from
Closed

Backport of Update server-acl-init to always check for the deployed serviceAccountToken secret into release/1.0.x #1905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7dc9f14
backport of commit fed11b770615baaca8c17625e62353f07a8a8ee1
kschoche Dec 5, 2022
249e32c
backport of commit 90ad3270f0ae3d110742fdcfa46aea6ef5daa343
Ranjandas Dec 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cli/cmd/install/install_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ func TestInstall(t *testing.T) {
},
messages: []string{
"\n==> Checking if Consul can be installed\n ✓ No existing Consul installations found.\n ✓ No existing Consul persistent volume claims found\n ✓ No existing Consul secrets found.\n",
"\n==> Consul Installation Summary\n Name: consul\n Namespace: consul\n \n Helm value overrides\n --------------------\n connectInject:\n enabled: true\n metrics:\n defaultEnableMerging: true\n defaultEnabled: true\n enableGatewayMetrics: true\n controller:\n enabled: true\n global:\n metrics:\n enableAgentMetrics: true\n enabled: true\n name: consul\n prometheus:\n enabled: true\n server:\n replicas: 1\n ui:\n enabled: true\n service:\n enabled: true\n \n",
"\n==> Consul Installation Summary\n Name: consul\n Namespace: consul\n \n Helm value overrides\n --------------------\n connectInject:\n enabled: true\n metrics:\n defaultEnableMerging: true\n defaultEnabled: true\n enableGatewayMetrics: true\n global:\n metrics:\n enableAgentMetrics: true\n enabled: true\n name: consul\n prometheus:\n enabled: true\n server:\n replicas: 1\n ui:\n enabled: true\n service:\n enabled: true\n \n",
"\n==> Installing Consul\n ✓ Downloaded charts.\n ✓ Consul installed in namespace \"consul\".\n",
},
helmActionsRunner: &helm.MockActionRunner{},
Expand All @@ -574,7 +574,7 @@ func TestInstall(t *testing.T) {
},
messages: []string{
"\n==> Checking if Consul can be installed\n ✓ No existing Consul installations found.\n ✓ No existing Consul persistent volume claims found\n ✓ No existing Consul secrets found.\n",
"\n==> Consul Installation Summary\n Name: consul\n Namespace: consul\n \n Helm value overrides\n --------------------\n connectInject:\n enabled: true\n controller:\n enabled: true\n global:\n acls:\n manageSystemACLs: true\n gossipEncryption:\n autoGenerate: true\n name: consul\n tls:\n enableAutoEncrypt: true\n enabled: true\n server:\n replicas: 1\n \n",
"\n==> Consul Installation Summary\n Name: consul\n Namespace: consul\n \n Helm value overrides\n --------------------\n connectInject:\n enabled: true\n global:\n acls:\n manageSystemACLs: true\n gossipEncryption:\n autoGenerate: true\n name: consul\n tls:\n enableAutoEncrypt: true\n enabled: true\n server:\n replicas: 1\n \n",
"\n==> Installing Consul\n ✓ Downloaded charts.\n ✓ Consul installed in namespace \"consul\".\n",
},
helmActionsRunner: &helm.MockActionRunner{},
Expand Down
4 changes: 2 additions & 2 deletions cli/cmd/status/status_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func TestStatus(t *testing.T) {
"status with servers returns success": {
input: []string{},
messages: []string{
fmt.Sprintf("\n==> Consul Status Summary\nName\tNamespace\tStatus\tChart Version\tAppVersion\tRevision\tLast Updated \n \t \tREADY \t1.0.0 \t \t0 \t%s\t\n", notImeStr),
fmt.Sprintf("\n==> Consul Status Summary\nName\tNamespace\tStatus\tChart Version\tAppVersion\tRevision\tLast Updated \n \t \tREADY \t1.0.0 \t \t0 \t%s\t\n", notImeStr),
"\n==> Config:\n {}\n \nConsul servers healthy 3/3\n",
},
preProcessingFunc: func(k8s kubernetes.Interface) error {
Expand All @@ -102,7 +102,7 @@ func TestStatus(t *testing.T) {
"status with pre-install and pre-upgrade hooks returns success and outputs hook status": {
input: []string{},
messages: []string{
fmt.Sprintf("\n==> Consul Status Summary\nName\tNamespace\tStatus\tChart Version\tAppVersion\tRevision\tLast Updated \n \t \tREADY \t1.0.0 \t \t0 \t%s\t\n", notImeStr),
fmt.Sprintf("\n==> Consul Status Summary\nName\tNamespace\tStatus\tChart Version\tAppVersion\tRevision\tLast Updated \n \t \tREADY \t1.0.0 \t \t0 \t%s\t\n", notImeStr),
"\n==> Config:\n {}\n \n",
"\n==> Status Of Helm Hooks:\npre-install-hook pre-install: Succeeded\npre-upgrade-hook pre-upgrade: Succeeded\nConsul servers healthy 3/3\n",
},
Expand Down
4 changes: 2 additions & 2 deletions cli/cmd/upgrade/upgrade_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ func TestUpgrade(t *testing.T) {
messages: []string{
"\n==> Checking if Consul can be upgraded\n ✓ Existing Consul installation found to be upgraded.\n Name: consul\n Namespace: consul\n",
"\n==> Checking if Consul demo application can be upgraded\n No existing Consul demo application installation found.\n",
"\n==> Consul Upgrade Summary\n ✓ Downloaded charts.\n \n Difference between user overrides for current and upgraded charts\n -----------------------------------------------------------------\n + connectInject:\n + enabled: true\n + metrics:\n + defaultEnableMerging: true\n + defaultEnabled: true\n + enableGatewayMetrics: true\n + controller:\n + enabled: true\n + global:\n + metrics:\n + enableAgentMetrics: true\n + enabled: true\n + name: consul\n + prometheus:\n + enabled: true\n + server:\n + replicas: 1\n + ui:\n + enabled: true\n + service:\n + enabled: true\n \n",
"\n==> Consul Upgrade Summary\n ✓ Downloaded charts.\n \n Difference between user overrides for current and upgraded charts\n -----------------------------------------------------------------\n + connectInject:\n + enabled: true\n + metrics:\n + defaultEnableMerging: true\n + defaultEnabled: true\n + enableGatewayMetrics: true\n + global:\n + metrics:\n + enableAgentMetrics: true\n + enabled: true\n + name: consul\n + prometheus:\n + enabled: true\n + server:\n + replicas: 1\n + ui:\n + enabled: true\n + service:\n + enabled: true\n \n",
"\n==> Upgrading Consul\n ✓ Consul upgraded in namespace \"consul\".\n",
},
helmActionsRunner: &helm.MockActionRunner{
Expand All @@ -477,7 +477,7 @@ func TestUpgrade(t *testing.T) {
messages: []string{
"\n==> Checking if Consul can be upgraded\n ✓ Existing Consul installation found to be upgraded.\n Name: consul\n Namespace: consul\n",
"\n==> Checking if Consul demo application can be upgraded\n No existing Consul demo application installation found.\n",
"\n==> Consul Upgrade Summary\n ✓ Downloaded charts.\n \n Difference between user overrides for current and upgraded charts\n -----------------------------------------------------------------\n + connectInject:\n + enabled: true\n + controller:\n + enabled: true\n + global:\n + acls:\n + manageSystemACLs: true\n + gossipEncryption:\n + autoGenerate: true\n + name: consul\n + tls:\n + enableAutoEncrypt: true\n + enabled: true\n + server:\n + replicas: 1\n \n",
"\n==> Consul Upgrade Summary\n ✓ Downloaded charts.\n \n Difference between user overrides for current and upgraded charts\n -----------------------------------------------------------------\n + connectInject:\n + enabled: true\n + global:\n + acls:\n + manageSystemACLs: true\n + gossipEncryption:\n + autoGenerate: true\n + name: consul\n + tls:\n + enableAutoEncrypt: true\n + enabled: true\n + server:\n + replicas: 1\n \n",
"\n==> Upgrading Consul\n ✓ Consul upgraded in namespace \"consul\".\n",
},
helmActionsRunner: &helm.MockActionRunner{
Expand Down
2 changes: 0 additions & 2 deletions cli/preset/demo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ connectInject:
enableGatewayMetrics: true
server:
replicas: 1
controller:
enabled: true
ui:
enabled: true
service:
Expand Down
2 changes: 0 additions & 2 deletions cli/preset/quickstart.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ connectInject:
enableGatewayMetrics: true
server:
replicas: 1
controller:
enabled: true
ui:
enabled: true
service:
Expand Down
2 changes: 0 additions & 2 deletions cli/preset/secure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ server:
replicas: 1
connectInject:
enabled: true
controller:
enabled: true
`

return config.ConvertToMap(values), nil
Expand Down
17 changes: 7 additions & 10 deletions control-plane/subcommand/server-acl-init/connect_inject.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,13 @@ func (c *Command) createAuthMethodTmpl(authMethodName string, useNS bool) (api.A

var saSecret *apiv1.Secret
var secretNames []string
if len(authMethodServiceAccount.Secrets) == 0 {
// 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"))
} else {
// ServiceAccounts always have a SecretRef in Kubernetes < 1.24. The Secret contains the JWT token.
for _, secretRef := range authMethodServiceAccount.Secrets {
secretNames = append(secretNames, secretRef.Name)
}
// 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"))
// ServiceAccounts always have a SecretRef in Kubernetes < 1.24. The Secret contains the JWT token.
for _, secretRef := range authMethodServiceAccount.Secrets {
secretNames = append(secretNames, secretRef.Name)
}
// Because there could be multiple secrets attached to the service account,
// we need pick the first one of type corev1.SecretTypeServiceAccountToken.
Expand Down
18 changes: 16 additions & 2 deletions control-plane/subcommand/server-acl-init/connect_inject_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,20 @@ func TestCommand_createAuthMethodTmpl_SecretNotFound(t *testing.T) {
ctx: ctx,
}

// create the auth method secret since it is always deployed by helm chart.
authMethodSecretName := resourcePrefix + "-auth-method"
secret := &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: authMethodSecretName,
Labels: map[string]string{common.CLILabelKey: common.CLILabelValue},
},
Data: map[string][]byte{},
// Make it not a service-account-token so the test can pass through to checking the other secrets.
Type: v1.SecretTypeOpaque,
}
_, err := k8s.CoreV1().Secrets(ns).Create(ctx, secret, metav1.CreateOptions{})
require.NoError(t, err)

serviceAccountName := resourcePrefix + "-auth-method"
secretName := resourcePrefix + "-connect-injector"

Expand All @@ -53,15 +67,15 @@ func TestCommand_createAuthMethodTmpl_SecretNotFound(t *testing.T) {
}

// Create a secret of non service-account-token type (we're using the opaque type).
secret := &v1.Secret{
secret = &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
Labels: map[string]string{common.CLILabelKey: common.CLILabelValue},
},
Data: map[string][]byte{},
Type: v1.SecretTypeOpaque,
}
_, err := k8s.CoreV1().Secrets(ns).Create(ctx, secret, metav1.CreateOptions{})
_, err = k8s.CoreV1().Secrets(ns).Create(ctx, secret, metav1.CreateOptions{})
require.NoError(t, err)

_, err = cmd.createAuthMethodTmpl("test", true)
Expand Down