Skip to content

Refactor stack_processor_utils.go template processing to reduce complexity #1575

@osterman

Description

@osterman

Describe the Bug

The template processing logic in internal/exec/stack_processor_utils.go:279 has high complexity (nestif: 6) and required a //nolint:nestif directive to pass linting. This indicates the function has deeply nested conditional logic that should be refactored.

Current State

File: internal/exec/stack_processor_utils.go
Line: 279
Function: Template processing in stack manifest imports
Complexity: 6 (nestif)

//nolint:nestif // Acceptable complexity for error handling with merge context
if !skipTemplatesProcessingInImports && len(context) > 0 {
    stackManifestTemplatesProcessed, err = ProcessTmpl(relativeFilePath, stackYamlConfig, context, ignoreMissingTemplateValues)
    if err != nil {
        if atmosConfig.Logs.Level == u.LogLevelTrace || atmosConfig.Logs.Level == u.LogLevelDebug {
            stackManifestTemplatesErrorMessage = fmt.Sprintf("\n\n%s", stackYamlConfig)
        }
        // Check if we have merge context to provide enhanced error formatting
        if mergeContext != nil {
            // Wrap the error with the sentinel first to preserve it
            wrappedErr := fmt.Errorf("%w: %v", errUtils.ErrInvalidStackManifest, err)
            // Then format it with context information
            e := mergeContext.FormatError(wrappedErr, fmt.Sprintf("stack manifest '%s'%s", relativeFilePath, stackManifestTemplatesErrorMessage))
            return nil, nil, nil, nil, nil, nil, nil, e
        } else {
            e := fmt.Errorf("%w: stack manifest '%s'\n%v%s", errUtils.ErrInvalidStackManifest, relativeFilePath, err, stackManifestTemplatesErrorMessage)
            return nil, nil, nil, nil, nil, nil, nil, e
        }
    }
}

Why This Needs Refactoring

  1. Testability: Deeply nested logic is harder to unit test in isolation
  2. Maintainability: Complex conditionals make future modifications error-prone
  3. Readability: Multiple levels of nesting reduce code clarity
  4. Code Quality: Linter warnings indicate code smell that should be addressed

Proposed Solution

Refactor the template processing logic into smaller, focused functions:

Example Approach

// processStackManifestTemplate processes Go templates in the imported stack manifest.
func processStackManifestTemplate(
    atmosConfig *schema.AtmosConfiguration,
    relativeFilePath string,
    stackYamlConfig string,
    context map[string]any,
    ignoreMissingTemplateValues bool,
    mergeContext *MergeContext,
) (string, error) {
    processed, err := ProcessTmpl(relativeFilePath, stackYamlConfig, context, ignoreMissingTemplateValues)
    if err != nil {
        return "", formatTemplateError(atmosConfig, relativeFilePath, stackYamlConfig, err, mergeContext)
    }
    return processed, nil
}

// formatTemplateError formats template processing errors with appropriate context.
func formatTemplateError(
    atmosConfig *schema.AtmosConfiguration,
    relativeFilePath string,
    stackYamlConfig string,
    err error,
    mergeContext *MergeContext,
) error {
    errorMessage := buildTemplateErrorMessage(atmosConfig, stackYamlConfig)
    
    if mergeContext != nil {
        return formatErrorWithMergeContext(mergeContext, relativeFilePath, errorMessage, err)
    }
    return formatErrorWithoutMergeContext(relativeFilePath, errorMessage, err)
}

// buildTemplateErrorMessage constructs the error message based on log level.
func buildTemplateErrorMessage(atmosConfig *schema.AtmosConfiguration, stackYamlConfig string) string {
    if atmosConfig.Logs.Level == u.LogLevelTrace || atmosConfig.Logs.Level == u.LogLevelDebug {
        return fmt.Sprintf("\n\n%s", stackYamlConfig)
    }
    return ""
}

// formatErrorWithMergeContext formats error using merge context.
func formatErrorWithMergeContext(
    mergeContext *MergeContext,
    relativeFilePath string,
    errorMessage string,
    err error,
) error {
    wrappedErr := fmt.Errorf("%w: %v", errUtils.ErrInvalidStackManifest, err)
    return mergeContext.FormatError(wrappedErr, fmt.Sprintf("stack manifest '%s'%s", relativeFilePath, errorMessage))
}

// formatErrorWithoutMergeContext formats error without merge context.
func formatErrorWithoutMergeContext(
    relativeFilePath string,
    errorMessage string,
    err error,
) error {
    return fmt.Errorf("%w: stack manifest '%s'\n%v%s", errUtils.ErrInvalidStackManifest, relativeFilePath, err, errorMessage)
}

Acceptance Criteria

  • Remove //nolint:nestif directive from line 279
  • Refactor template processing into smaller, single-purpose functions
  • Each extracted function should have a clear, testable responsibility
  • Add unit tests for each new helper function
  • Ensure existing tests continue to pass
  • golangci-lint passes without suppressions
  • Code coverage maintained or improved

Additional Context

This issue was identified during PR #1504 when adding Windows CI path normalization fixes. The nolint directive was added as a temporary measure to unblock the PR, but the underlying complexity issue should be addressed.

Related

  • Commit: 7ba97f3
  • File: internal/exec/stack_processor_utils.go
  • Lines: 279-297

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions