From b742e82d693cfa5d37b2058b815fcfcf2c19a00d Mon Sep 17 00:00:00 2001 From: David Laubreiter Date: Wed, 18 Oct 2023 14:00:17 +0200 Subject: [PATCH] refactor: Slit account stuff in its own file This change allows nice restructuring. Additionally, introduced unit tests for that file, and errors to easily check what happened. --- pkg/manifest/account.go | 97 ++++++++++++++++ pkg/manifest/account_test.go | 188 ++++++++++++++++++++++++++++++++ pkg/manifest/manifest_loader.go | 64 +---------- 3 files changed, 288 insertions(+), 61 deletions(-) create mode 100644 pkg/manifest/account.go create mode 100644 pkg/manifest/account_test.go diff --git a/pkg/manifest/account.go b/pkg/manifest/account.go new file mode 100644 index 0000000000..c343ad529f --- /dev/null +++ b/pkg/manifest/account.go @@ -0,0 +1,97 @@ +/* + * @license + * Copyright 2023 Dynatrace LLC + * 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 manifest + +import ( + "errors" + "fmt" + "github.com/google/uuid" +) + +var ( + errNameMissing = errors.New("name is missing") + errAccUidMissing = errors.New("accountUUID is missing") +) + +type invalidUUIDError struct { + uuid string + err error +} + +func (e invalidUUIDError) Error() string { + return fmt.Sprintf("invalid uuid %q: %s", e.uuid, e.err) +} + +func (e invalidUUIDError) Unwrap() error { + return e.err +} + +func convertSingleAccount(c *LoaderContext, a account) (Account, error) { + + if a.AccountUUID == "" { + return Account{}, errAccUidMissing + } + + accountId, err := uuid.Parse(a.AccountUUID) + if err != nil { + return Account{}, invalidUUIDError{a.AccountUUID, err} + } + + oAuthDef, err := parseOAuth(c, a.OAuth) + if err != nil { + return Account{}, fmt.Errorf("oAuth is invalid: %w", err) + } + + var urlDef *URLDefinition + if a.ApiUrl != nil { + if u, err := parseURLDefinition(c, *a.ApiUrl); err != nil { + return Account{}, fmt.Errorf("apiUrl: %w", err) + } else { + urlDef = &u + } + } + + acc := Account{ + Name: a.Name, + AccountUUID: accountId, + ApiUrl: urlDef, + OAuth: oAuthDef, + } + + return acc, nil +} + +// convertAccounts converts the persistence definition to the in-memory definition +func convertAccounts(c *LoaderContext, accounts []account) (map[string]Account, error) { + + result := make(map[string]Account, len(accounts)) + + for i, a := range accounts { + if a.Name == "" { + return nil, fmt.Errorf("failed to parse account on position %d: %w", i, errNameMissing) + } + + acc, err := convertSingleAccount(c, a) + if err != nil { + return nil, fmt.Errorf("failed to parse account %q: %w", a.Name, err) + } + + result[acc.Name] = acc + } + + return result, nil +} diff --git a/pkg/manifest/account_test.go b/pkg/manifest/account_test.go new file mode 100644 index 0000000000..eb8e30e2f6 --- /dev/null +++ b/pkg/manifest/account_test.go @@ -0,0 +1,188 @@ +//go:build unit + +/* + * @license + * Copyright 2023 Dynatrace LLC + * 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 manifest + +import ( + "encoding/json" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestValidAccounts(t *testing.T) { + t.Setenv("SECRET", "secret") + acc := account{ + Name: "name", + AccountUUID: uuid.New().String(), + ApiUrl: &url{ + Value: "https://example.com", + }, + OAuth: oAuth{ + ClientID: authSecret{ + Name: "SECRET", + }, + ClientSecret: authSecret{ + Name: "SECRET", + }, + TokenEndpoint: &url{ + Value: "https://example.com", + }, + }, + } + + // account 2 has no api name + acc2 := account{ + Name: "name2", + AccountUUID: uuid.New().String(), + OAuth: oAuth{ + ClientID: authSecret{ + Name: "SECRET", + }, + ClientSecret: authSecret{ + Name: "SECRET", + }, + TokenEndpoint: nil, + }, + } + + v, err := convertAccounts(&LoaderContext{}, []account{acc, acc2}) + assert.NoError(t, err) + + assert.Equal(t, v, map[string]Account{ + "name": { + Name: "name", + AccountUUID: uuid.MustParse(acc.AccountUUID), + ApiUrl: &URLDefinition{ + Type: ValueURLType, + Value: "https://example.com", + }, + OAuth: OAuth{ + ClientID: AuthSecret{Name: "SECRET", Value: "secret"}, + ClientSecret: AuthSecret{Name: "SECRET", Value: "secret"}, + TokenEndpoint: &URLDefinition{ + Type: ValueURLType, + Value: "https://example.com", + }, + }, + }, + "name2": { + Name: "name2", + AccountUUID: uuid.MustParse(acc2.AccountUUID), + ApiUrl: nil, + OAuth: OAuth{ + ClientID: AuthSecret{Name: "SECRET", Value: "secret"}, + ClientSecret: AuthSecret{Name: "SECRET", Value: "secret"}, + TokenEndpoint: nil, + }, + }, + }) + +} + +func TestInvalidAccounts(t *testing.T) { + t.Setenv("SECRET", "secret") + + // default account to permute + validAccount := account{ + Name: "name", + AccountUUID: uuid.New().String(), + ApiUrl: &url{ + Value: "https://example.com", + }, + OAuth: oAuth{ + ClientID: authSecret{ + Name: "SECRET", + }, + ClientSecret: authSecret{ + Name: "SECRET", + }, + TokenEndpoint: &url{ + Value: "https://example.com", + }, + }, + } + + // validate that the default is valid + _, err := convertAccounts(&LoaderContext{}, []account{validAccount}) + assert.NoError(t, err) + + // tests + t.Run("name is missing", func(t *testing.T) { + a := validAccount + a.Name = "" + + _, err := convertAccounts(&LoaderContext{}, []account{a}) + assert.ErrorIs(t, err, errNameMissing) + }) + + t.Run("accountUUID is missing", func(t *testing.T) { + a := validAccount + a.AccountUUID = "" + + _, err := convertAccounts(&LoaderContext{}, []account{a}) + assert.ErrorIs(t, err, errAccUidMissing) + }) + + t.Run("accountUUID is invalid", func(t *testing.T) { + a := deepCopy(t, validAccount) + a.AccountUUID = "this-is-not-a-valid-uuid" + + _, err := convertAccounts(&LoaderContext{}, []account{a}) + uuidErr := invalidUUIDError{} + if assert.ErrorAs(t, err, &uuidErr) { + assert.Equal(t, uuidErr.uuid, "this-is-not-a-valid-uuid") + } + }) + + t.Run("oAuth is set", func(t *testing.T) { + a := deepCopy(t, validAccount) + a.OAuth = oAuth{} + + _, err := convertAccounts(&LoaderContext{}, []account{a}) + assert.ErrorContains(t, err, "oAuth is invalid") + }) + + t.Run("oAuth.id is not set", func(t *testing.T) { + a := deepCopy(t, validAccount) + a.OAuth.ClientID = authSecret{} + + _, err := convertAccounts(&LoaderContext{}, []account{a}) + assert.ErrorContains(t, err, "ClientID: no name given or empty") + + }) + + t.Run("oAuth.secret is not set", func(t *testing.T) { + a := deepCopy(t, validAccount) + a.OAuth.ClientSecret = authSecret{} + + _, err := convertAccounts(&LoaderContext{}, []account{a}) + assert.ErrorContains(t, err, "ClientSecret: no name given or empty") + }) +} + +// deepCopy marshals and then marshals the payload, thus only works for public members, thus only private spaced +func deepCopy(t *testing.T, in account) account { + d, e := json.Marshal(in) + assert.NoError(t, e) + + var o account + e = json.Unmarshal(d, &o) + assert.NoError(t, e) + return o +} diff --git a/pkg/manifest/manifest_loader.go b/pkg/manifest/manifest_loader.go index 1df1fb1d63..9b65cd66ad 100644 --- a/pkg/manifest/manifest_loader.go +++ b/pkg/manifest/manifest_loader.go @@ -24,7 +24,6 @@ import ( "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/slices" version2 "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/version" "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/version" - "github.com/google/uuid" "github.com/spf13/afero" "gopkg.in/yaml.v2" "os" @@ -172,9 +171,9 @@ func LoadManifest(context *LoaderContext) (Manifest, []error) { } // accounts - accounts, accErrs := convertAccounts(context, manifestYAML.Accounts) - if len(accErrs) > 0 { - errs = append(errs, accErrs...) + accounts, accErr := convertAccounts(context, manifestYAML.Accounts) + if accErr != nil { + errs = append(errs, newManifestLoaderError(context.ManifestPath, accErr.Error())) } // if any errors occurred up to now, return them @@ -189,63 +188,6 @@ func LoadManifest(context *LoaderContext) (Manifest, []error) { }, nil } -// convertAccounts converts the persistence definition to the in-memory definition -func convertAccounts(c *LoaderContext, accounts []account) (map[string]Account, []error) { - - var errs []error - result := make(map[string]Account, len(accounts)) - - // validate - for i, a := range accounts { - if a.Name == "" { - errs = append(errs, fmt.Errorf("failed to parse account on position %d: 'name' is missing", i)) - continue - } - - if a.AccountUUID == "" { - errs = append(errs, fmt.Errorf("failed to parse account %q: accountUUID is missing", a.Name)) - continue - } - - accountId, err := uuid.Parse(a.AccountUUID) - if err != nil { - errs = append(errs, fmt.Errorf("failed to parse account %q: accountUUID is invalid: %w", a.Name, err)) - continue - } - - oAuth, err := parseOAuth(c, a.OAuth) - if err != nil { - errs = append(errs, fmt.Errorf("failed to parse account %q: oAuth: %w", a.Name, err)) - continue - } - - var url *URLDefinition - if a.ApiUrl != nil { - if u, err := parseURLDefinition(c, *a.ApiUrl); err != nil { - errs = append(errs, fmt.Errorf("failed to parse account %q: apiUrl: %w", a.Name, err)) - continue - } else { - url = &u - } - } - - acc := Account{ - Name: a.Name, - AccountUUID: accountId, - ApiUrl: url, - OAuth: oAuth, - } - - result[acc.Name] = acc - } - - if len(errs) > 0 { - return nil, errs - } - - return result, nil -} - func parseAuth(context *LoaderContext, a auth) (Auth, error) { token, err := parseAuthSecret(context, a.Token) if err != nil {