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

@@ -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
+}

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
+}

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 {