🐛 gitlab: Fallback to wildcard variables and use pagination (bugfix) (#1838)

* gitlab: fallback to wildcard variables when using "GetAllSecrets"

Signed-off-by: Dominik Zeiger <dominik@zeiger.biz>

pkg/provider/gitlab/fake/fake.go

@@ -14,9 +14,32 @@ limitations under the License.
 package fake
 
 import (
+	"net/http"
+
 	gitlab "github.com/xanzy/go-gitlab"
 )
 
+type APIResponse[O any] struct {
+	Output   O
+	Response *gitlab.Response
+	Error    error
+}
+
+type GitVariable interface {
+	gitlab.ProjectVariable |
+		gitlab.GroupVariable
+}
+
+type extractKey[V GitVariable] func(gv V) string
+
+func keyFromProjectVariable(pv gitlab.ProjectVariable) string {
+	return pv.Key
+}
+
+func keyFromGroupVariable(gv gitlab.GroupVariable) string {
+	return gv.Key
+}
+
 type GitlabMockProjectsClient struct {
 	listProjectsGroups func(pid interface{}, opt *gitlab.ListProjectGroupOptions, options ...gitlab.RequestOptionFunc) ([]*gitlab.ProjectGroup, *gitlab.Response, error)
 }
@@ -46,15 +69,63 @@ func (mc *GitlabMockProjectVariablesClient) ListVariables(pid interface{}, opt *
 	return mc.listVariables(pid)
 }
 
-func (mc *GitlabMockProjectVariablesClient) WithValue(output *gitlab.ProjectVariable, response *gitlab.Response, err error) {
+func (mc *GitlabMockProjectVariablesClient) WithValue(response APIResponse[[]*gitlab.ProjectVariable]) {
-	if mc != nil {
+	mc.WithValues([]APIResponse[[]*gitlab.ProjectVariable]{response})
-		mc.getVariable = func(pid interface{}, key string, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectVariable, *gitlab.Response, error) {
+}
-			return output, response, err
-		}
 
-		mc.listVariables = func(pid interface{}, options ...gitlab.RequestOptionFunc) ([]*gitlab.ProjectVariable, *gitlab.Response, error) {
+func (mc *GitlabMockProjectVariablesClient) WithValues(responses []APIResponse[[]*gitlab.ProjectVariable]) {
-			return []*gitlab.ProjectVariable{output}, response, err
+	if mc != nil {
+		mc.getVariable = mockGetVariable(keyFromProjectVariable, responses)
+		mc.listVariables = mockListVariable(responses)
+	}
+}
+
+func mockGetVariable[V GitVariable](keyExtractor extractKey[V], responses []APIResponse[[]*V]) func(interface{}, string, ...gitlab.RequestOptionFunc) (*V, *gitlab.Response, error) {
+	getCount := -1
+	return func(pid interface{}, key string, options ...gitlab.RequestOptionFunc) (*V, *gitlab.Response, error) {
+		getCount++
+		if getCount > len(responses)-1 {
+			return nil, make404APIResponse(), nil
 		}
+		var match *V
+		for _, v := range responses[getCount].Output {
+			if keyExtractor(*v) == key {
+				match = v
+			}
+		}
+		if match == nil {
+			return nil, make404APIResponse(), nil
+		}
+		return match, responses[getCount].Response, responses[getCount].Error
+	}
+}
+
+func mockListVariable[V GitVariable](responses []APIResponse[[]*V]) func(interface{}, ...gitlab.RequestOptionFunc) ([]*V, *gitlab.Response, error) {
+	listCount := -1
+	return func(pid interface{}, options ...gitlab.RequestOptionFunc) ([]*V, *gitlab.Response, error) {
+		listCount++
+		if listCount > len(responses)-1 {
+			return nil, makeAPIResponse(listCount, len(responses)), nil
+		}
+		return responses[listCount].Output, responses[listCount].Response, responses[listCount].Error
+	}
+}
+
+func make404APIResponse() *gitlab.Response {
+	return &gitlab.Response{
+		Response: &http.Response{
+			StatusCode: http.StatusNotFound,
+		},
+	}
+}
+
+func makeAPIResponse(page, pages int) *gitlab.Response {
+	return &gitlab.Response{
+		Response: &http.Response{
+			StatusCode: http.StatusOK,
+		},
+		CurrentPage: page,
+		TotalPages:  pages,
 	}
 }
 
@@ -82,3 +153,10 @@ func (mc *GitlabMockGroupVariablesClient) WithValue(output *gitlab.GroupVariable
 		}
 	}
 }
+
+func (mc *GitlabMockGroupVariablesClient) WithValues(responses []APIResponse[[]*gitlab.GroupVariable]) {
+	if mc != nil {
+		mc.getVariable = mockGetVariable(keyFromGroupVariable, responses)
+		mc.listVariables = mockListVariable(responses)
+	}
+}

pkg/provider/gitlab/gitlab.go

@@ -23,7 +23,7 @@ import (
 	"strings"
 
 	"github.com/tidwall/gjson"
-	gitlab "github.com/xanzy/go-gitlab"
+	"github.com/xanzy/go-gitlab"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -237,34 +237,51 @@ func (g *Gitlab) GetAllSecrets(ctx context.Context, ref esv1beta1.ExternalSecret
 		return nil, err
 	}
 
+	var gopts = &gitlab.ListGroupVariablesOptions{PerPage: 100}
 	secretData := make(map[string][]byte)
 	for _, groupID := range g.groupIDs {
-		var groupVars []*gitlab.GroupVariable
+		for groupPage := 1; ; groupPage++ {
-		groupVars, _, err := g.groupVariablesClient.ListVariables(groupID, nil)
+			gopts.Page = groupPage
+			groupVars, response, err := g.groupVariablesClient.ListVariables(groupID, gopts)
+			if err != nil {
+				return nil, err
+			}
+			for _, data := range groupVars {
+				matching, key, isWildcard := matchesFilter(g.environment, data.EnvironmentScope, data.Key, matcher)
+				if !matching && !isWildcard {
+					continue
+				}
+				secretData[key] = []byte(data.Value)
+			}
+			if response.CurrentPage >= response.TotalPages {
+				break
+			}
+		}
+	}
+
+	var popts = &gitlab.ListProjectVariablesOptions{PerPage: 100}
+	for projectPage := 1; ; projectPage++ {
+		popts.Page = projectPage
+		projectData, response, err := g.projectVariablesClient.ListVariables(g.projectID, popts)
 		if err != nil {
 			return nil, err
 		}
-		for _, data := range groupVars {
+
-			matching, key := matchesFilter(g.environment, data.EnvironmentScope, data.Key, matcher)
+		for _, data := range projectData {
+			matching, key, isWildcard := matchesFilter(g.environment, data.EnvironmentScope, data.Key, matcher)
+
 			if !matching {
 				continue
 			}
+			_, exists := secretData[key]
+			if exists && isWildcard {
+				continue
+			}
 			secretData[key] = []byte(data.Value)
 		}
-	}
+		if response.CurrentPage >= response.TotalPages {
-
+			break
-	var projectData []*gitlab.ProjectVariable
-	projectData, _, err = g.projectVariablesClient.ListVariables(g.projectID, nil)
-	if err != nil {
-		return nil, err
-	}
-
-	for _, data := range projectData {
-		matching, key := matchesFilter(g.environment, data.EnvironmentScope, data.Key, matcher)
-		if !matching {
-			continue
 		}
-		secretData[key] = []byte(data.Value)
 	}
 
 	return secretData, nil
@@ -389,19 +406,20 @@ func isEmptyOrWildcard(environment string) bool {
 	return environment == "" || environment == "*"
 }
 
-func matchesFilter(environment, varEnvironment, key string, matcher *find.Matcher) (bool, string) {
+func matchesFilter(environment, varEnvironment, key string, matcher *find.Matcher) (bool, string, bool) {
-	if !isEmptyOrWildcard(environment) {
+	isWildcard := isEmptyOrWildcard(varEnvironment)
+	if !isWildcard && !isEmptyOrWildcard(environment) {
 		// as of now gitlab does not support filtering of EnvironmentScope through the api call
 		if varEnvironment != environment {
-			return false, ""
+			return false, "", isWildcard
 		}
 	}
 
 	if key == "" || (matcher != nil && !matcher.MatchName(key)) {
-		return false, ""
+		return false, "", isWildcard
 	}
 
-	return true, key
+	return true, key, isWildcard
 }
 
 func (g *Gitlab) Close(ctx context.Context) error {

pkg/provider/gitlab/gitlab_test.go

@@ -41,11 +41,13 @@ const (
 	username              = "user-name"
 	userkey               = "user-key"
 	environment           = "prod"
+	environmentTest       = "test"
 	projectvalue          = "projectvalue"
 	groupvalue            = "groupvalue"
 	groupid               = "groupId"
 	defaultErrorMessage   = "[%d] unexpected error: [%s], expected: [%s]"
 	errMissingCredentials = "credentials are empty"
+	testKey               = "testKey"
 	findTestPrefix        = "test.*"
 )
 
@@ -58,8 +60,10 @@ type secretManagerTestCase struct {
 	apiInputEnv              string
 	projectAPIOutput         *gitlab.ProjectVariable
 	projectAPIResponse       *gitlab.Response
+	projectAPIOutputs        []*fakegitlab.APIResponse[[]*gitlab.ProjectVariable]
 	projectGroupsAPIOutput   []*gitlab.ProjectGroup
 	projectGroupsAPIResponse *gitlab.Response
+	groupAPIOutputs          []*fakegitlab.APIResponse[[]*gitlab.GroupVariable]
 	groupAPIOutput           *gitlab.GroupVariable
 	groupAPIResponse         *gitlab.Response
 	ref                      *esv1beta1.ExternalSecretDataRemoteRef
@@ -99,14 +103,14 @@ func makeValidSecretManagerTestCase() *secretManagerTestCase {
 		expectedValidationResult: esv1beta1.ValidationResultReady,
 		expectedData:             map[string][]byte{},
 	}
-	smtc.mockProjectVarClient.WithValue(smtc.projectAPIOutput, smtc.projectAPIResponse, smtc.apiErr)
+	prepareMockProjectVarClient(&smtc)
-	smtc.mockGroupVarClient.WithValue(smtc.groupAPIOutput, smtc.groupAPIResponse, smtc.apiErr)
+	prepareMockGroupVarClient(&smtc)
 	return &smtc
 }
 
 func makeValidRef() *esv1beta1.ExternalSecretDataRemoteRef {
 	return &esv1beta1.ExternalSecretDataRemoteRef{
-		Key:     "test-secret",
+		Key:     testKey,
 		Version: "default",
 	}
 }
@@ -134,7 +138,7 @@ func makeValidAPIInputProjectID() string {
 }
 
 func makeValidAPIInputKey() string {
-	return "testKey"
+	return testKey
 }
 
 func makeValidEnvironment() string {
@@ -146,6 +150,8 @@ func makeValidProjectAPIResponse() *gitlab.Response {
 		Response: &http.Response{
 			StatusCode: http.StatusOK,
 		},
+		CurrentPage: 1,
+		TotalPages:  1,
 	}
 }
 
@@ -154,6 +160,8 @@ func makeValidProjectGroupsAPIResponse() *gitlab.Response {
 		Response: &http.Response{
 			StatusCode: http.StatusOK,
 		},
+		CurrentPage: 1,
+		TotalPages:  1,
 	}
 }
 
@@ -162,12 +170,14 @@ func makeValidGroupAPIResponse() *gitlab.Response {
 		Response: &http.Response{
 			StatusCode: http.StatusOK,
 		},
+		CurrentPage: 1,
+		TotalPages:  1,
 	}
 }
 
 func makeValidProjectAPIOutput() *gitlab.ProjectVariable {
 	return &gitlab.ProjectVariable{
-		Key:              "testKey",
+		Key:              testKey,
 		Value:            "",
 		EnvironmentScope: environment,
 	}
@@ -203,8 +213,8 @@ func makeValidSecretManagerTestCaseCustom(tweaks ...func(smtc *secretManagerTest
 		fn(smtc)
 	}
 	smtc.mockProjectsClient.WithValue(smtc.projectGroupsAPIOutput, smtc.projectGroupsAPIResponse, smtc.apiErr)
-	smtc.mockProjectVarClient.WithValue(smtc.projectAPIOutput, smtc.projectAPIResponse, smtc.apiErr)
+	prepareMockProjectVarClient(smtc)
-	smtc.mockGroupVarClient.WithValue(smtc.groupAPIOutput, smtc.groupAPIResponse, smtc.apiErr)
+	prepareMockGroupVarClient(smtc)
 	return smtc
 }
 
@@ -215,12 +225,33 @@ func makeValidSecretManagerGetAllTestCaseCustom(tweaks ...func(smtc *secretManag
 	for _, fn := range tweaks {
 		fn(smtc)
 	}
-	smtc.mockProjectVarClient.WithValue(smtc.projectAPIOutput, smtc.projectAPIResponse, smtc.apiErr)
+	prepareMockProjectVarClient(smtc)
-	smtc.mockGroupVarClient.WithValue(smtc.groupAPIOutput, smtc.groupAPIResponse, smtc.apiErr)
+	prepareMockGroupVarClient(smtc)
-
 	return smtc
 }
 
+func prepareMockProjectVarClient(smtc *secretManagerTestCase) {
+	responses := make([]fakegitlab.APIResponse[[]*gitlab.ProjectVariable], 0)
+	if smtc.projectAPIOutput != nil {
+		responses = append(responses, fakegitlab.APIResponse[[]*gitlab.ProjectVariable]{Output: []*gitlab.ProjectVariable{smtc.projectAPIOutput}, Response: smtc.projectAPIResponse, Error: smtc.apiErr})
+	}
+	for _, response := range smtc.projectAPIOutputs {
+		responses = append(responses, *response)
+	}
+	smtc.mockProjectVarClient.WithValues(responses)
+}
+
+func prepareMockGroupVarClient(smtc *secretManagerTestCase) {
+	responses := make([]fakegitlab.APIResponse[[]*gitlab.GroupVariable], 0)
+	if smtc.projectAPIOutput != nil {
+		responses = append(responses, fakegitlab.APIResponse[[]*gitlab.GroupVariable]{Output: []*gitlab.GroupVariable{smtc.groupAPIOutput}, Response: smtc.groupAPIResponse, Error: smtc.apiErr})
+	}
+	for _, response := range smtc.groupAPIOutputs {
+		responses = append(responses, *response)
+	}
+	smtc.mockGroupVarClient.WithValues(responses)
+}
+
 // This case can be shared by both GetSecret and GetSecretMap tests.
 // bad case: set apiErr.
 var setAPIErr = func(smtc *secretManagerTestCase) {
@@ -380,14 +411,14 @@ func TestGetSecret(t *testing.T) {
 	}
 	groupSecretProjectOverride := func(smtc *secretManagerTestCase) {
 		smtc.projectAPIOutput.Value = projectvalue
-		smtc.groupAPIOutput.Key = "testkey"
+		smtc.groupAPIOutput.Key = testKey
 		smtc.groupAPIOutput.Value = groupvalue
 		smtc.expectedSecret = smtc.projectAPIOutput.Value
 	}
 	groupWithoutProjectOverride := func(smtc *secretManagerTestCase) {
 		smtc.groupIDs = []string{groupid}
 		smtc.projectAPIResponse.Response.StatusCode = 404
-		smtc.groupAPIOutput.Key = "testkey"
+		smtc.groupAPIOutput.Key = testKey
 		smtc.groupAPIOutput.Value = groupvalue
 		smtc.expectedSecret = smtc.groupAPIOutput.Value
 	}
@@ -452,7 +483,7 @@ func TestGetAllSecrets(t *testing.T) {
 	}
 	setMatchingSecretFindString := func(smtc *secretManagerTestCase) {
 		smtc.projectAPIOutput = &gitlab.ProjectVariable{
-			Key:              "testkey",
+			Key:              testKey,
 			Value:            projectvalue,
 			EnvironmentScope: environment,
 		}
@@ -461,25 +492,25 @@ func TestGetAllSecrets(t *testing.T) {
 	}
 	setNoMatchingRegexpFindString := func(smtc *secretManagerTestCase) {
 		smtc.projectAPIOutput = &gitlab.ProjectVariable{
-			Key:              "testkey",
+			Key:              testKey,
 			Value:            projectvalue,
-			EnvironmentScope: "test",
+			EnvironmentScope: environmentTest,
 		}
 		smtc.expectedSecret = ""
 		smtc.refFind.Name = makeFindName("foo.*")
 	}
 	setUnmatchedEnvironmentFindString := func(smtc *secretManagerTestCase) {
 		smtc.projectAPIOutput = &gitlab.ProjectVariable{
-			Key:              "testkey",
+			Key:              testKey,
 			Value:            projectvalue,
-			EnvironmentScope: "test",
+			EnvironmentScope: environmentTest,
 		}
 		smtc.expectedSecret = ""
 		smtc.refFind.Name = makeFindName(findTestPrefix)
 	}
 	setMatchingSecretFindTags := func(smtc *secretManagerTestCase) {
 		smtc.projectAPIOutput = &gitlab.ProjectVariable{
-			Key:              "testkey",
+			Key:              testKey,
 			Value:            projectvalue,
 			EnvironmentScope: environment,
 		}
@@ -492,6 +523,108 @@ func TestGetAllSecrets(t *testing.T) {
 		smtc.expectError = "'find.tags' is constrained by 'environment_scope' of the store"
 		smtc.refFind.Tags = map[string]string{"environment_scope": environment}
 	}
+	setWildcardDoesntOverwriteEnvironmentValue := func(smtc *secretManagerTestCase) {
+		var1 := gitlab.ProjectVariable{
+			Key:              testKey,
+			Value:            "wildcardValue",
+			EnvironmentScope: "*",
+		}
+		var2 := gitlab.ProjectVariable{
+			Key:              testKey,
+			Value:            "expectedValue",
+			EnvironmentScope: environmentTest,
+		}
+		var3 := gitlab.ProjectVariable{
+			Key:              testKey,
+			Value:            "wildcardValue",
+			EnvironmentScope: "*",
+		}
+		vars := []*gitlab.ProjectVariable{&var1, &var2, &var3}
+		smtc.projectAPIOutputs = []*fakegitlab.APIResponse[[]*gitlab.ProjectVariable]{{Output: vars, Response: smtc.projectAPIResponse, Error: nil}}
+		smtc.projectAPIOutput = nil
+		smtc.apiInputEnv = environmentTest
+		smtc.expectedSecret = "expectedValue"
+		smtc.refFind.Name = makeFindName(findTestPrefix)
+	}
+	setFilterByEnvironmentWithWildcard := func(smtc *secretManagerTestCase) {
+		var1 := gitlab.ProjectVariable{
+			Key:              testKey,
+			Value:            projectvalue,
+			EnvironmentScope: "*",
+		}
+		var2 := gitlab.ProjectVariable{
+			Key:              "testKey2",
+			Value:            "value2",
+			EnvironmentScope: environment,
+		}
+		var3 := gitlab.ProjectVariable{
+			Key:              "testKey3",
+			Value:            "value3",
+			EnvironmentScope: environmentTest,
+		}
+		var4 := gitlab.ProjectVariable{
+			Key:              "anotherKey4",
+			Value:            "value4",
+			EnvironmentScope: environment,
+		}
+		vars := []*gitlab.ProjectVariable{&var1, &var2, &var3, &var4}
+		smtc.projectAPIOutput = nil
+		smtc.projectAPIOutputs = []*fakegitlab.APIResponse[[]*gitlab.ProjectVariable]{{Output: vars, Response: smtc.projectAPIResponse, Error: nil}}
+		smtc.apiInputEnv = environment
+		smtc.expectedData = map[string][]byte{testKey: []byte(projectvalue), "testKey2": []byte("value2")}
+		smtc.refFind.Name = makeFindName(findTestPrefix)
+	}
+	setPaginationInGroupAndProjectVars := func(smtc *secretManagerTestCase) {
+		smtc.groupIDs = []string{groupid}
+		gvar1 := gitlab.GroupVariable{
+			Key:              testKey + "Group",
+			Value:            "groupValue1",
+			EnvironmentScope: environmentTest,
+		}
+		gvar2 := gitlab.GroupVariable{
+			Key:              testKey,
+			Value:            "groupValue2",
+			EnvironmentScope: environmentTest,
+		}
+		pvar1 := gitlab.ProjectVariable{
+			Key:              testKey,
+			Value:            "testValue1",
+			EnvironmentScope: environmentTest,
+		}
+		pvar2a := gitlab.ProjectVariable{
+			Key:              testKey + "2a",
+			Value:            "testValue2a",
+			EnvironmentScope: environmentTest,
+		}
+		pvar2b := gitlab.ProjectVariable{
+			Key:              testKey + "2b",
+			Value:            "testValue2b",
+			EnvironmentScope: environmentTest,
+		}
+		gPage1 := []*gitlab.GroupVariable{&gvar1}
+		gResponsePage1 := makeValidGroupAPIResponse()
+		gResponsePage1.TotalPages = 2
+		gResponsePage1.CurrentPage = 1
+		gPage2 := []*gitlab.GroupVariable{&gvar2}
+		gResponsePage2 := makeValidGroupAPIResponse()
+		gResponsePage2.TotalPages = 2
+		gResponsePage2.CurrentPage = 1
+		pPage1 := []*gitlab.ProjectVariable{&pvar1}
+		pResponsePage1 := makeValidProjectAPIResponse()
+		pResponsePage1.TotalPages = 2
+		pResponsePage1.CurrentPage = 1
+		pPage2 := []*gitlab.ProjectVariable{&pvar2a, &pvar2b}
+		pResponsePage2 := makeValidProjectAPIResponse()
+		pResponsePage2.TotalPages = 2
+		pResponsePage2.CurrentPage = 2
+		smtc.groupAPIOutputs = []*fakegitlab.APIResponse[[]*gitlab.GroupVariable]{{Output: gPage1, Response: gResponsePage1, Error: nil}, {Output: gPage2, Response: gResponsePage2, Error: nil}}
+		smtc.groupAPIOutput = nil
+		smtc.projectAPIOutputs = []*fakegitlab.APIResponse[[]*gitlab.ProjectVariable]{{Output: pPage1, Response: pResponsePage1, Error: nil}, {Output: pPage2, Response: pResponsePage2, Error: nil}}
+		smtc.projectAPIOutput = nil
+		smtc.apiInputEnv = environmentTest
+		smtc.expectedData = map[string][]byte{testKey: []byte("testValue1"), "testKey2a": []byte("testValue2a"), "testKey2b": []byte("testValue2b"), "testKeyGroup": []byte("groupValue1")}
+		smtc.refFind.Name = makeFindName(findTestPrefix)
+	}
 
 	cases := []*secretManagerTestCase{
 		makeValidSecretManagerGetAllTestCaseCustom(setMissingFindRegex),
@@ -501,7 +634,10 @@ func TestGetAllSecrets(t *testing.T) {
 		makeValidSecretManagerGetAllTestCaseCustom(setNoMatchingRegexpFindString),
 		makeValidSecretManagerGetAllTestCaseCustom(setUnmatchedEnvironmentFindString),
 		makeValidSecretManagerGetAllTestCaseCustom(setMatchingSecretFindTags),
+		makeValidSecretManagerGetAllTestCaseCustom(setWildcardDoesntOverwriteEnvironmentValue),
 		makeValidSecretManagerGetAllTestCaseCustom(setEnvironmentConstrainedByStore),
+		makeValidSecretManagerGetAllTestCaseCustom(setFilterByEnvironmentWithWildcard),
+		makeValidSecretManagerGetAllTestCaseCustom(setPaginationInGroupAndProjectVars),
 		makeValidSecretManagerGetAllTestCaseCustom(setAPIErr),
 		makeValidSecretManagerGetAllTestCaseCustom(setNilMockClient),
 	}
@@ -511,12 +647,16 @@ func TestGetAllSecrets(t *testing.T) {
 		sm.environment = v.apiInputEnv
 		sm.projectVariablesClient = v.mockProjectVarClient
 		sm.groupVariablesClient = v.mockGroupVarClient
+		sm.groupIDs = v.groupIDs
+		if v.expectedSecret != "" {
+			v.expectedData = map[string][]byte{testKey: []byte(v.expectedSecret)}
+		}
 		out, err := sm.GetAllSecrets(context.Background(), *v.refFind)
 		if !ErrorContains(err, v.expectError) {
 			t.Errorf(defaultErrorMessage, k, err.Error(), v.expectError)
 		}
-		if v.expectError == "" && string(out[v.projectAPIOutput.Key]) != v.expectedSecret {
+		if err == nil && !reflect.DeepEqual(out, v.expectedData) {
-			t.Errorf("[%d] unexpected secret: [%s], expected [%s]", k, string(out[v.projectAPIOutput.Key]), v.expectedSecret)
+			t.Errorf("[%d] unexpected secret data: [%#v], expected [%#v]", k, out, v.expectedData)
 		}
 	}
 }
@@ -533,7 +673,7 @@ func TestGetAllSecretsWithGroups(t *testing.T) {
 		smtc.groupIDs = []string{groupid}
 		smtc.projectAPIOutput.Value = projectvalue
 		smtc.groupAPIOutput.Value = groupvalue
-		smtc.expectedData = map[string][]byte{"testKey": []byte(projectvalue), "groupKey": []byte(groupvalue)}
+		smtc.expectedData = map[string][]byte{testKey: []byte(projectvalue), "groupKey": []byte(groupvalue)}
 		smtc.refFind.Name = makeFindName(".*Key")
 	}
 	groupAndOverrideProjectSecrets := func(smtc *secretManagerTestCase) {
@@ -541,16 +681,16 @@ func TestGetAllSecretsWithGroups(t *testing.T) {
 		smtc.projectAPIOutput.Value = projectvalue
 		smtc.groupAPIOutput.Key = smtc.projectAPIOutput.Key
 		smtc.groupAPIOutput.Value = groupvalue
-		smtc.expectedData = map[string][]byte{"testKey": []byte(projectvalue)}
+		smtc.expectedData = map[string][]byte{testKey: []byte(projectvalue)}
 		smtc.refFind.Name = makeFindName(".*Key")
 	}
 	groupAndProjectWithDifferentEnvSecrets := func(smtc *secretManagerTestCase) {
 		smtc.groupIDs = []string{groupid}
 		smtc.projectAPIOutput.Value = projectvalue
-		smtc.projectAPIOutput.EnvironmentScope = "test"
+		smtc.projectAPIOutput.EnvironmentScope = environmentTest
 		smtc.groupAPIOutput.Key = smtc.projectAPIOutput.Key
 		smtc.groupAPIOutput.Value = groupvalue
-		smtc.expectedData = map[string][]byte{"testKey": []byte(groupvalue)}
+		smtc.expectedData = map[string][]byte{testKey: []byte(groupvalue)}
 		smtc.refFind.Name = makeFindName(".*Key")
 	}
 
@@ -562,7 +702,7 @@ func TestGetAllSecretsWithGroups(t *testing.T) {
 	}
 
 	sm := Gitlab{}
-	sm.environment = "prod"
+	sm.environment = environment
 	for k, v := range cases {
 		sm.projectVariablesClient = v.mockProjectVarClient
 		sm.groupVariablesClient = v.mockGroupVarClient