From 102c6898d5a75e39860812dba6e93d3937f320aa Mon Sep 17 00:00:00 2001 From: Marvin Beckers Date: Mon, 6 Dec 2021 10:26:54 +0100 Subject: [PATCH 1/5] Add variables for application credentials and validate them Signed-off-by: Marvin Beckers --- pkg/credentials/credentials.go | 88 ++++++++++++++++++++++++---------- 1 file changed, 62 insertions(+), 26 deletions(-) diff --git a/pkg/credentials/credentials.go b/pkg/credentials/credentials.go index dc7609e9d..45d42306d 100644 --- a/pkg/credentials/credentials.go +++ b/pkg/credentials/credentials.go @@ -31,27 +31,29 @@ import ( // The environment variable names with credential in them const ( // Variables that KubeOne (and Terraform) expect to see - AWSAccessKeyID = "AWS_ACCESS_KEY_ID" - AWSSecretAccessKey = "AWS_SECRET_ACCESS_KEY" //nolint:gosec - AzureClientID = "ARM_CLIENT_ID" - AzureClientSecret = "ARM_CLIENT_SECRET" //nolint:gosec - AzureTenantID = "ARM_TENANT_ID" - AzureSubscribtionID = "ARM_SUBSCRIPTION_ID" - DigitalOceanTokenKey = "DIGITALOCEAN_TOKEN" - GoogleServiceAccountKey = "GOOGLE_CREDENTIALS" - HetznerTokenKey = "HCLOUD_TOKEN" - OpenStackAuthURL = "OS_AUTH_URL" - OpenStackDomainName = "OS_DOMAIN_NAME" - OpenStackPassword = "OS_PASSWORD" - OpenStackRegionName = "OS_REGION_NAME" - OpenStackTenantID = "OS_TENANT_ID" - OpenStackTenantName = "OS_TENANT_NAME" - OpenStackUserName = "OS_USERNAME" - PacketAPIKey = "PACKET_AUTH_TOKEN" //nolint:gosec - PacketProjectID = "PACKET_PROJECT_ID" - VSphereAddress = "VSPHERE_SERVER" - VSpherePassword = "VSPHERE_PASSWORD" - VSphereUsername = "VSPHERE_USER" + AWSAccessKeyID = "AWS_ACCESS_KEY_ID" + AWSSecretAccessKey = "AWS_SECRET_ACCESS_KEY" //nolint:gosec + AzureClientID = "ARM_CLIENT_ID" + AzureClientSecret = "ARM_CLIENT_SECRET" //nolint:gosec + AzureTenantID = "ARM_TENANT_ID" + AzureSubscribtionID = "ARM_SUBSCRIPTION_ID" + DigitalOceanTokenKey = "DIGITALOCEAN_TOKEN" + GoogleServiceAccountKey = "GOOGLE_CREDENTIALS" + HetznerTokenKey = "HCLOUD_TOKEN" + OpenStackAuthURL = "OS_AUTH_URL" + OpenStackDomainName = "OS_DOMAIN_NAME" + OpenStackPassword = "OS_PASSWORD" + OpenStackRegionName = "OS_REGION_NAME" + OpenStackTenantID = "OS_TENANT_ID" + OpenStackTenantName = "OS_TENANT_NAME" + OpenStackUserName = "OS_USERNAME" + OpenStackApplicationCredentialID = "OS_APPLICATION_CREDENTIAL_ID" + OpenStackApplicationCredentialSecret = "OS_APPLICATION_CREDENTIAL_SECRET" + PacketAPIKey = "PACKET_AUTH_TOKEN" //nolint:gosec + PacketProjectID = "PACKET_PROJECT_ID" + VSphereAddress = "VSPHERE_SERVER" + VSpherePassword = "VSPHERE_PASSWORD" + VSphereUsername = "VSPHERE_USER" // Variables that machine-controller expects AzureClientIDMC = "AZURE_CLIENT_ID" @@ -158,6 +160,8 @@ func ProviderCredentials(cloudProvider kubeone.CloudProviderSpec, credentialsFil {Name: OpenStackAuthURL}, {Name: OpenStackUserName, MachineControllerName: OpenStackUserNameMC}, {Name: OpenStackPassword}, + {Name: OpenStackApplicationCredentialID, MachineControllerName: OpenStackUserNameMC}, + {Name: OpenStackApplicationCredentialSecret}, {Name: OpenStackDomainName}, {Name: OpenStackRegionName}, {Name: OpenStackTenantID}, @@ -283,13 +287,45 @@ func defaultValidationFunc(creds map[string]string) error { } func openstackValidationFunc(creds map[string]string) error { - for k, v := range creds { - if k == OpenStackTenantID || k == OpenStackTenantName { - continue + alwaysRequired := []string{OpenStackAuthURL, OpenStackDomainName, OpenStackRegionName} + + for _, key := range alwaysRequired { + if v, ok := creds[key]; !ok || len(v) == 0 { + return errors.Errorf("key %v is required but is not present", key) } - if len(v) == 0 { - return errors.Errorf("key %v is required but isn't present", k) + } + + var ( + appCredsInUse bool + userCredsInUse bool + ) + + if v, ok := creds[OpenStackApplicationCredentialID]; ok && len(v) != 0 { + if v, ok := creds[OpenStackApplicationCredentialSecret]; !ok || len(v) == 0 { + return errors.Errorf("key %v is required if %v is present", OpenStackApplicationCredentialSecret, OpenStackApplicationCredentialID) } + appCredsInUse = true + } + + if v, ok := creds[OpenStackUserName]; !appCredsInUse && ok && len(v) != 0 { + if v, ok := creds[OpenStackPassword]; !ok || len(v) == 0 { + return errors.Errorf("key %v is required but is not present", OpenStackPassword) + } + userCredsInUse = true + } + + if !appCredsInUse && !userCredsInUse { + return errors.Errorf("no app credentials (%s, %s) or user credentials (%s, %s) found", + OpenStackApplicationCredentialID, OpenStackApplicationCredentialSecret, + OpenStackUserName, OpenStackPassword, + ) + } + + if appCredsInUse && userCredsInUse { + return errors.Errorf("both app credentials (%s, %s) and user credentials (%s, %s) defined", + OpenStackApplicationCredentialID, OpenStackApplicationCredentialSecret, + OpenStackUserName, OpenStackPassword, + ) } if v, ok := creds[OpenStackTenantID]; !ok || len(v) == 0 { From 5d059eafb9a84f43c56807d2af1185c174434ffd Mon Sep 17 00:00:00 2001 From: Marvin Beckers Date: Mon, 6 Dec 2021 11:40:10 +0100 Subject: [PATCH 2/5] Remove MachineControllerName field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Marko Mudrinić --- pkg/credentials/credentials.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/credentials/credentials.go b/pkg/credentials/credentials.go index 45d42306d..10681b4a7 100644 --- a/pkg/credentials/credentials.go +++ b/pkg/credentials/credentials.go @@ -160,7 +160,7 @@ func ProviderCredentials(cloudProvider kubeone.CloudProviderSpec, credentialsFil {Name: OpenStackAuthURL}, {Name: OpenStackUserName, MachineControllerName: OpenStackUserNameMC}, {Name: OpenStackPassword}, - {Name: OpenStackApplicationCredentialID, MachineControllerName: OpenStackUserNameMC}, + {Name: OpenStackApplicationCredentialID}, {Name: OpenStackApplicationCredentialSecret}, {Name: OpenStackDomainName}, {Name: OpenStackRegionName}, From 80c4321e34e9a2e9ef3aa3d596337e0d88620e81 Mon Sep 17 00:00:00 2001 From: Marvin Beckers Date: Mon, 6 Dec 2021 14:05:56 +0100 Subject: [PATCH 3/5] Handle both 'partial credentials' situations and add test cases Signed-off-by: Marvin Beckers --- pkg/credentials/credentials.go | 50 +++++---- pkg/credentials/credentials_test.go | 157 ++++++++++++++++++++++++++++ 2 files changed, 187 insertions(+), 20 deletions(-) create mode 100644 pkg/credentials/credentials_test.go diff --git a/pkg/credentials/credentials.go b/pkg/credentials/credentials.go index 10681b4a7..efbdf8e4a 100644 --- a/pkg/credentials/credentials.go +++ b/pkg/credentials/credentials.go @@ -296,36 +296,46 @@ func openstackValidationFunc(creds map[string]string) error { } var ( - appCredsInUse bool - userCredsInUse bool + appCredsIdOkay bool + appCredsSecretOkay bool + userCredsUsernameOkay bool + userCredsPasswordOkay bool ) if v, ok := creds[OpenStackApplicationCredentialID]; ok && len(v) != 0 { - if v, ok := creds[OpenStackApplicationCredentialSecret]; !ok || len(v) == 0 { - return errors.Errorf("key %v is required if %v is present", OpenStackApplicationCredentialSecret, OpenStackApplicationCredentialID) - } - appCredsInUse = true + appCredsIdOkay = true } - if v, ok := creds[OpenStackUserName]; !appCredsInUse && ok && len(v) != 0 { - if v, ok := creds[OpenStackPassword]; !ok || len(v) == 0 { - return errors.Errorf("key %v is required but is not present", OpenStackPassword) - } - userCredsInUse = true + if v, ok := creds[OpenStackApplicationCredentialSecret]; ok && len(v) != 0 { + appCredsSecretOkay = true } - if !appCredsInUse && !userCredsInUse { - return errors.Errorf("no app credentials (%s, %s) or user credentials (%s, %s) found", - OpenStackApplicationCredentialID, OpenStackApplicationCredentialSecret, - OpenStackUserName, OpenStackPassword, - ) + if v, ok := creds[OpenStackUserName]; ok && len(v) != 0 { + userCredsUsernameOkay = true } - if appCredsInUse && userCredsInUse { - return errors.Errorf("both app credentials (%s, %s) and user credentials (%s, %s) defined", + if v, ok := creds[OpenStackPassword]; ok && len(v) != 0 { + userCredsPasswordOkay = true + } + + if (appCredsIdOkay || appCredsSecretOkay) && (userCredsUsernameOkay || userCredsPasswordOkay) { + return errors.Errorf("both app credentials (%s %s) and user credentials (%s %s) found", OpenStackApplicationCredentialID, OpenStackApplicationCredentialSecret, - OpenStackUserName, OpenStackPassword, - ) + OpenStackUserName, OpenStackPassword) + } + + if (appCredsIdOkay && !appCredsSecretOkay) || (!appCredsIdOkay && appCredsSecretOkay) { + return errors.Errorf("only one of %s, %s is set for application credentials", + OpenStackApplicationCredentialID, OpenStackApplicationCredentialSecret) + } + + if (userCredsUsernameOkay && !userCredsPasswordOkay) || (!userCredsUsernameOkay && userCredsPasswordOkay) { + return errors.Errorf("only one of %s, %s is set for user credentials", + OpenStackUserName, OpenStackPassword) + } + + if (!appCredsIdOkay && !appCredsSecretOkay) && (!userCredsUsernameOkay && !userCredsPasswordOkay) { + return errors.New("no valid credentials (either application or user) found") } if v, ok := creds[OpenStackTenantID]; !ok || len(v) == 0 { diff --git a/pkg/credentials/credentials_test.go b/pkg/credentials/credentials_test.go new file mode 100644 index 000000000..b01e3cdf6 --- /dev/null +++ b/pkg/credentials/credentials_test.go @@ -0,0 +1,157 @@ +/* +Copyright 2021 The KubeOne Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package credentials + +import ( + "errors" + "testing" +) + +func TestOpenstackValidationFunc(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + creds map[string]string + err error + }{ + { + name: "no-credentials", + creds: map[string]string{}, + err: errors.New("key OS_AUTH_URL is required but is not present"), + }, + { + name: "application-credentials", + creds: map[string]string{ + "OS_TENANT_NAME": "test", + "OS_AUTH_URL": "https://localhost:5000", + "OS_DOMAIN_NAME": "test", + "OS_REGION_NAME": "de", + "OS_APPLICATION_CREDENTIAL_ID": "1234", + "OS_APPLICATION_CREDENTIAL_SECRET": "5678", + }, + err: nil, + }, + { + name: "no-credential-secret", + creds: map[string]string{ + "OS_TENANT_NAME": "test", + "OS_AUTH_URL": "https://localhost:5000", + "OS_DOMAIN_NAME": "test", + "OS_REGION_NAME": "de", + "OS_APPLICATION_CREDENTIAL_ID": "1234", + }, + err: errors.New("only one of OS_APPLICATION_CREDENTIAL_ID, OS_APPLICATION_CREDENTIAL_SECRET is set for application credentials"), + }, + { + name: "no-credential-id", + creds: map[string]string{ + "OS_TENANT_NAME": "test", + "OS_AUTH_URL": "https://localhost:5000", + "OS_DOMAIN_NAME": "test", + "OS_REGION_NAME": "de", + "OS_APPLICATION_CREDENTIAL_SECRET": "5678", + }, + err: errors.New("only one of OS_APPLICATION_CREDENTIAL_ID, OS_APPLICATION_CREDENTIAL_SECRET is set for application credentials"), + }, + { + name: "user-credentials", + creds: map[string]string{ + "OS_TENANT_NAME": "test", + "OS_AUTH_URL": "https://localhost:5000", + "OS_DOMAIN_NAME": "test", + "OS_REGION_NAME": "de", + "OS_USERNAME": "1234", + "OS_PASSWORD": "5678", + }, + err: nil, + }, + { + name: "no-password", + creds: map[string]string{ + "OS_TENANT_NAME": "test", + "OS_AUTH_URL": "https://localhost:5000", + "OS_DOMAIN_NAME": "test", + "OS_REGION_NAME": "de", + "OS_USERNAME": "1234", + }, + err: errors.New("only one of OS_USERNAME, OS_PASSWORD is set for user credentials"), + }, + { + name: "no-username", + creds: map[string]string{ + "OS_TENANT_NAME": "test", + "OS_AUTH_URL": "https://localhost:5000", + "OS_DOMAIN_NAME": "test", + "OS_REGION_NAME": "de", + "OS_PASSWORD": "5678", + }, + err: errors.New("only one of OS_USERNAME, OS_PASSWORD is set for user credentials"), + }, + { + name: "mixed-credentials-1", + creds: map[string]string{ + "OS_TENANT_NAME": "test", + "OS_AUTH_URL": "https://localhost:5000", + "OS_DOMAIN_NAME": "test", + "OS_REGION_NAME": "de", + "OS_APPLICATION_CREDENTIAL_ID": "1234", + "OS_PASSWORD": "5678", + }, + err: errors.New("both app credentials (OS_APPLICATION_CREDENTIAL_ID OS_APPLICATION_CREDENTIAL_SECRET) and user credentials (OS_USERNAME OS_PASSWORD) found"), + }, + { + name: "mixed-credentials-2", + creds: map[string]string{ + "OS_TENANT_NAME": "test", + "OS_AUTH_URL": "https://localhost:5000", + "OS_DOMAIN_NAME": "test", + "OS_REGION_NAME": "de", + "OS_APPLICATION_CREDENTIAL_SECRET": "5678", + "OS_USERNAME": "1234", + }, + err: errors.New("both app credentials (OS_APPLICATION_CREDENTIAL_ID OS_APPLICATION_CREDENTIAL_SECRET) and user credentials (OS_USERNAME OS_PASSWORD) found"), + }, + { + name: "mixed-credentials-3", + creds: map[string]string{ + "OS_TENANT_NAME": "test", + "OS_AUTH_URL": "https://localhost:5000", + "OS_DOMAIN_NAME": "test", + "OS_REGION_NAME": "de", + "OS_APPLICATION_CREDENTIAL_ID": "1234", + "OS_APPLICATION_CREDENTIAL_SECRET": "5678", + "OS_USERNAME": "1234", + "OS_PASSWORD": "5678", + }, + err: errors.New("both app credentials (OS_APPLICATION_CREDENTIAL_ID OS_APPLICATION_CREDENTIAL_SECRET) and user credentials (OS_USERNAME OS_PASSWORD) found"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := openstackValidationFunc(tt.creds) + if tt.err != nil && err != nil { + if err.Error() != tt.err.Error() { + t.Errorf("expected error = '%v', got error = '%v'", tt.err.Error(), err.Error()) + } + } else if err != tt.err { + t.Errorf("%s: expected error = %v, got error = %v", tt.name, tt.err, err) + } + }) + } +} From cb6da6b2212d76a64995c9e0ecc0c86178403b35 Mon Sep 17 00:00:00 2001 From: Marvin Beckers Date: Mon, 6 Dec 2021 14:14:52 +0100 Subject: [PATCH 4/5] Make the linter happy Signed-off-by: Marvin Beckers --- pkg/credentials/credentials.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/credentials/credentials.go b/pkg/credentials/credentials.go index efbdf8e4a..2a7cd1853 100644 --- a/pkg/credentials/credentials.go +++ b/pkg/credentials/credentials.go @@ -296,14 +296,14 @@ func openstackValidationFunc(creds map[string]string) error { } var ( - appCredsIdOkay bool + appCredsIDOkay bool appCredsSecretOkay bool userCredsUsernameOkay bool userCredsPasswordOkay bool ) if v, ok := creds[OpenStackApplicationCredentialID]; ok && len(v) != 0 { - appCredsIdOkay = true + appCredsIDOkay = true } if v, ok := creds[OpenStackApplicationCredentialSecret]; ok && len(v) != 0 { @@ -318,13 +318,13 @@ func openstackValidationFunc(creds map[string]string) error { userCredsPasswordOkay = true } - if (appCredsIdOkay || appCredsSecretOkay) && (userCredsUsernameOkay || userCredsPasswordOkay) { + if (appCredsIDOkay || appCredsSecretOkay) && (userCredsUsernameOkay || userCredsPasswordOkay) { return errors.Errorf("both app credentials (%s %s) and user credentials (%s %s) found", OpenStackApplicationCredentialID, OpenStackApplicationCredentialSecret, OpenStackUserName, OpenStackPassword) } - if (appCredsIdOkay && !appCredsSecretOkay) || (!appCredsIdOkay && appCredsSecretOkay) { + if (appCredsIDOkay && !appCredsSecretOkay) || (!appCredsIDOkay && appCredsSecretOkay) { return errors.Errorf("only one of %s, %s is set for application credentials", OpenStackApplicationCredentialID, OpenStackApplicationCredentialSecret) } @@ -334,7 +334,7 @@ func openstackValidationFunc(creds map[string]string) error { OpenStackUserName, OpenStackPassword) } - if (!appCredsIdOkay && !appCredsSecretOkay) && (!userCredsUsernameOkay && !userCredsPasswordOkay) { + if (!appCredsIDOkay && !appCredsSecretOkay) && (!userCredsUsernameOkay && !userCredsPasswordOkay) { return errors.New("no valid credentials (either application or user) found") } From 881e77d96b2d95e8413ddded6e0f9afd61343ce1 Mon Sep 17 00:00:00 2001 From: Marvin Beckers Date: Mon, 6 Dec 2021 14:25:30 +0100 Subject: [PATCH 5/5] Further disagreements with the linter Signed-off-by: Marvin Beckers --- .golangci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index fcb12ed72..08f419a65 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -60,3 +60,6 @@ issues: - path: pkg/scripts text: "`registry` always receives `\"127.0.0.1:5000\"`" + + - path: pkg/credentials + text: "cyclomatic complexity 32 of func `openstackValidationFunc` is high"