Refactor: move dangling functions onto SecretGroup

cyberark · Nov 3, 2021 · 49a5d0d · 49a5d0d
1 parent 29dcbfe
commit 49a5d0d
Showing 1 changed file with 95 additions and 104 deletions.
diff --git a/pkg/secrets/pushtofile/secret_group.go b/pkg/secrets/pushtofile/secret_group.go
@@ -30,17 +30,17 @@ type SecretGroup struct {
 
 // ResolvedSecretSpecs resolves all of the secret paths for a secret
 // group by prepending each path with that group's policy path prefix.
-func (s *SecretGroup) ResolvedSecretSpecs() []SecretSpec {
-	if len(s.PolicyPathPrefix) == 0 {
-		return s.SecretSpecs
+func (sg *SecretGroup) ResolvedSecretSpecs() []SecretSpec {
+	if len(sg.PolicyPathPrefix) == 0 {
+		return sg.SecretSpecs
 	}
 
-	specs := make([]SecretSpec, len(s.SecretSpecs))
-	copy(specs, s.SecretSpecs)
+	specs := make([]SecretSpec, len(sg.SecretSpecs))
+	copy(specs, sg.SecretSpecs)
 
 	// Update specs with policy path prefix
 	for i := range specs {
-		specs[i].Path = strings.TrimSuffix(s.PolicyPathPrefix, "/") +
+		specs[i].Path = strings.TrimSuffix(sg.PolicyPathPrefix, "/") +
 			"/" +
 			strings.TrimPrefix(specs[i].Path, "/")
 	}
@@ -50,16 +50,16 @@ func (s *SecretGroup) ResolvedSecretSpecs() []SecretSpec {
 
 // PushToFile uses the configuration on a secret group to inject secrets into a template
 // and write the result to a file.
-func (s *SecretGroup) PushToFile(secrets []*Secret) error {
-	return s.pushToFileWithDeps(openFileAsWriteCloser, pushToWriter, secrets)
+func (sg *SecretGroup) PushToFile(secrets []*Secret) error {
+	return sg.pushToFileWithDeps(openFileAsWriteCloser, pushToWriter, secrets)
 }
 
-func (s *SecretGroup) pushToFileWithDeps(
+func (sg *SecretGroup) pushToFileWithDeps(
 	depOpenWriteCloser openWriteCloserFunc,
 	depPushToWriter pushToWriterFunc,
 	secrets []*Secret) error {
 	// Make sure all the secret specs are accounted for
-	err := validateSecretsAgainstSpecs(secrets, s.SecretSpecs)
+	err := validateSecretsAgainstSpecs(secrets, sg.SecretSpecs)
 	if err != nil {
 		return err
 	}
@@ -69,16 +69,16 @@ func (s *SecretGroup) pushToFileWithDeps(
 	// 2. File format
 	// 3. Secret specs (user to validate file template)
 	fileTemplate, err := maybeFileTemplateFromFormat(
-		s.FileTemplate,
-		s.FileFormat,
-		s.SecretSpecs,
+		sg.FileTemplate,
+		sg.FileFormat,
+		sg.SecretSpecs,
 	)
 	if err != nil {
 		return err
 	}
 
 	//// Open and push to file
-	wc, err := depOpenWriteCloser(s.FilePath, s.FilePermissions)
+	wc, err := depOpenWriteCloser(sg.FilePath, sg.FilePermissions)
 	if err != nil {
 		return err
 	}
@@ -88,12 +88,83 @@ func (s *SecretGroup) pushToFileWithDeps(
 
 	return depPushToWriter(
 		wc,
-		s.Name,
+		sg.Name,
 		fileTemplate,
 		secrets,
 	)
 }
 
+func (sg *SecretGroup) absoluteFilePath(secretsBasePath string) (string, error) {
+	groupName := sg.Name
+	filePath := sg.FilePath
+	fileTemplate := sg.FileTemplate
+	fileFormat := sg.FileFormat
+
+	// filePath must be relative
+	if path.IsAbs(filePath) {
+		return "", fmt.Errorf(
+			"provided filepath %q for secret group %q is absolute, requires relative path",
+			filePath, groupName,
+		)
+	}
+
+	filePathIsDir := strings.HasSuffix(filePath, "/")
+
+	// fileTemplate requires filePath to point to a file (not a directory)
+	if filePathIsDir && len(fileTemplate) > 0 {
+		return "", fmt.Errorf(
+			"provided filepath %q for secret group %q must specify a path to a file, without a trailing %q",
+			filePath, groupName, "/",
+		)
+	}
+	// Without the restrictions of fileTemplate, the filename defaults to "{groupName}.{fileFormat}"
+	if filePathIsDir && len(fileTemplate) == 0 {
+		filePath = path.Join(
+			filePath,
+			fmt.Sprintf("%s.%s", groupName, fileFormat),
+		)
+	}
+
+	absoluteFilePath := path.Join(secretsBasePath, filePath)
+
+	// filePath must be relative to secrets base path. This protects against relative paths
+	// that, by using the double-dot path segment, resolve to a path that is not relative
+	// to the base path.
+	if !strings.HasPrefix(absoluteFilePath, secretsBasePath) {
+		return "", fmt.Errorf(
+			"provided filepath %q for secret group %q must be relative to secrets base path",
+			filePath, groupName,
+		)
+	}
+
+	return absoluteFilePath, nil
+}
+
+func (sg *SecretGroup) validate() []error {
+	groupName := sg.Name
+	fileFormat := sg.FileFormat
+	secretSpecs := sg.SecretSpecs
+
+	if errors := validateSecretPaths(secretSpecs, groupName); len(errors) > 0 {
+		return errors
+	}
+
+	if len(fileFormat) > 0 {
+		_, err := FileTemplateForFormat(fileFormat, secretSpecs)
+		if err != nil {
+			return []error{
+				fmt.Errorf(
+					"unable to process file format %q for group: %s",
+					fileFormat,
+					err,
+				),
+			}
+		}
+	}
+
+	return nil
+}
+
 func validateSecretsAgainstSpecs(
 	secrets []*Secret,
 	specs []SecretSpec,
@@ -207,106 +278,26 @@ func newSecretGroup(groupName string, secretsBasePath string, annotations map[st
 		return nil, []error{err}
 	}
 
-	errors := validateGroup(
-		groupName,
-		fileFormat,
-		secretSpecs,
-	)
-	if len(errors) > 0 {
-		return nil, errors
-	}
-
-	// Validate and generate absolute file path for group
-	filePath, err = absoluteFilePathForGroup(
-		groupName,
-		secretsBasePath,
-		filePath,
-		fileTemplate,
-		fileFormat,
-	)
-	if err != nil {
-		return nil, []error{err}
-	}
-
-	return &SecretGroup{
+	sg := &SecretGroup{
 		Name:             groupName,
 		FilePath:         filePath,
 		FileTemplate:     fileTemplate,
 		FileFormat:       fileFormat,
 		FilePermissions:  defaultFilePermissions,
 		PolicyPathPrefix: policyPathPrefix,
 		SecretSpecs:      secretSpecs,
-	}, nil
-}
-
-func validateGroup(
-	groupName string,
-	fileFormat string,
-	secretSpecs []SecretSpec,
-) []error {
-	if errors := validateSecretPaths(secretSpecs, groupName); len(errors) > 0 {
-		return errors
 	}
 
-	if len(fileFormat) > 0 {
-		_, err := FileTemplateForFormat(fileFormat, secretSpecs)
-		if err != nil {
-			return []error{
-				fmt.Errorf(
-					"unable to process file format annotation %q for group: %s",
-					fileFormat,
-					err,
-				),
-			}
-		}
-	}
-
-	return nil
-}
-
-func absoluteFilePathForGroup(
-	groupName string,
-	secretsBasePath string,
-	filePath string,
-	fileTemplate string,
-	fileFormat string,
-) (string, error) {
-	// filePath must be relative
-	if path.IsAbs(filePath) {
-		return "", fmt.Errorf(
-			"provided filepath %q for secret group %q is absolute, requires relative path",
-			filePath, groupName,
-		)
-	}
-
-	filePathIsDir := strings.HasSuffix(filePath, "/")
-
-	// fileTemplate requires filePath to point to a file (not a directory)
-	if filePathIsDir && len(fileTemplate) > 0 {
-		return "", fmt.Errorf(
-			"provided filepath %q for secret group %q must specify a path to a file (without a trailing %q), required when %q is configured",
-			filePath, groupName, "/", secretGroupFileTemplatePrefix+"{groupName}",
-		)
-	}
-	// Without the restrictions of fileTemplate, the filename defaults to "{groupName}.{fileFormat}"
-	if filePathIsDir && len(fileTemplate) == 0 {
-		filePath = path.Join(
-			filePath,
-			fmt.Sprintf("%s.%s", groupName, fileFormat),
-		)
+	errors := sg.validate()
+	if len(errors) > 0 {
+		return nil, errors
 	}
 
-	absoluteFilePath := path.Join(secretsBasePath, filePath)
-
-	// filePath must be relative to secrets base path. This protects against relative paths
-	// that, by using the double-dot path segment, resolve to a path that is not relative
-	// to the base path.
-	if !strings.HasPrefix(absoluteFilePath, secretsBasePath) {
-		return "", fmt.Errorf(
-			"provided filepath %q for secret group %q must be relative to secrets base path",
-			filePath, groupName,
-		)
+	// Generate absolute file path
+	sg.FilePath, err = sg.absoluteFilePath(secretsBasePath)
+	if err != nil {
+		return nil, []error{err}
 	}
 
-	return absoluteFilePath, nil
+	return sg, nil
 }