-
Notifications
You must be signed in to change notification settings - Fork 18
Add environment-based variable replacement for stacks #128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
sanzoghenzo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What a great feature!
It looks OK overall.
Some of the comments come from my personal preference, so they are open for debate 😉
I got two more observations (that can be addressed in future PRs):
- docker compose also supports envrionment variable substitution by just specifying an empty item in the
environmentmap (so you can avoid repeating the variable name in the value), should we support this, too? - Should we also support a global
environments_file? I'm thinking, for instance, of aDOMAINvariable that could be used across all the stacks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personal preference, but this AI generated doc seems too verbose to describe a relatively simple concept.
Please also add a link to this document in the main readme file.
| currentEnvironment = env | ||
| logger.Info("detected environment from node label", "environment", currentEnvironment, "label", environmentLabel) | ||
| } else { | ||
| logger.Warn("environment label not found on manager node, all stacks will be deployed", "label", environmentLabel) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel this should be a Warning, since this feature is an opt-in
| cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) | ||
| if err != nil { | ||
| return fmt.Errorf("could not create docker client for environment detection: %w", err) | ||
| } | ||
| defer cli.Close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to not use the dockerCli object here?
|
|
||
| // Check if this is a swarm manager | ||
| if !info.Swarm.ControlAvailable { | ||
| return fmt.Errorf("this node is not a swarm manager, cannot detect environment") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The node in which SwarmCD runs could be different from the managed node if a user sets a remote url in DOCKER_HOST or uses a socket proxy, so the "this node" wording could cause a bit of confusion.
| err = initDockerCli() | ||
| err = detectEnvironment() | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe an error of detectEnvironment should stop SwarmCD, especially if a user doesn't need this feature.
| shouldDeploy, err := swarmStack.loadEnvironmentVars() | ||
| if err != nil { | ||
| return | ||
| } | ||
| if !shouldDeploy { | ||
| log.Info("stack skipped due to environment filtering") | ||
| return revision, nil | ||
| } | ||
|
|
||
| stackBytes, err := swarmStack.readStack() | ||
| if err != nil { | ||
| return | ||
| } | ||
|
|
||
| if swarmStack.valuesFile != "" { | ||
| log.Debug("rendering template...") | ||
| stackBytes, err = swarmStack.renderComposeTemplate(stackBytes) | ||
| if err != nil { | ||
| return | ||
| } | ||
| } | ||
|
|
||
| stackContents, err := swarmStack.parseStackString([]byte(stackBytes)) | ||
| if err != nil { | ||
| return | ||
| } | ||
|
|
||
| log.Debug("parsing stack content...") | ||
| stackContents, err := swarmStack.parseStackString([]byte(stackBytes)) | ||
| err = swarmStack.replaceEnvVars(stackContents) | ||
| if err != nil { | ||
| return | ||
| } | ||
|
|
||
| log.Debug("decrypting secrets...") | ||
| err = swarmStack.decryptSopsFiles(stackContents) | ||
| if err != nil { | ||
| return "", fmt.Errorf("failed to decrypt one or more sops files for %s stack: %w", swarmStack.name, err) | ||
| } | ||
|
|
||
| log.Debug("rotating configs and secrets...") | ||
| err = swarmStack.rotateConfigsAndSecrets(stackContents) | ||
| if err != nil { | ||
| return | ||
| } | ||
|
|
||
| log.Debug("writing stack to file...") | ||
| err = swarmStack.writeStack(stackContents) | ||
| if err != nil { | ||
| return | ||
| } | ||
|
|
||
| log.Debug("deploying stack...") | ||
| err = swarmStack.deployStack() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to remove the debug messages? if you don't want to see them, just raise the log level 😉
| // If environments file is specified but no environment label is set on the node, | ||
| // this stack is environment-filtered and should NOT be deployed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the log message explains this already, no need for a comment here
| envFilePath := path.Join(swarmStack.repo.path, swarmStack.environmentsFile) | ||
| envFileBytes, err := os.ReadFile(envFilePath) | ||
| if err != nil { | ||
| // If file doesn't exist, it's not an error - just skip loading |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the comment
| // Current environment is not defined in the environments file | ||
| // This means this stack should not be deployed in this environment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove comment
| "available_environments", getKeys(envConfig.Environments)) | ||
| return false, nil | ||
| } | ||
| } | ||
|
|
||
| func getKeys(m map[string]map[string]string) []string { | ||
| keys := make([]string, 0, len(m)) | ||
| for k := range m { | ||
| keys = append(keys, k) | ||
| } | ||
| return keys | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a go guru, but it seems that this can be simplified with
| "available_environments", getKeys(envConfig.Environments)) | |
| return false, nil | |
| } | |
| } | |
| func getKeys(m map[string]map[string]string) []string { | |
| keys := make([]string, 0, len(m)) | |
| for k := range m { | |
| keys = append(keys, k) | |
| } | |
| return keys | |
| } | |
| "available_environments", slices.Collect(maps.Keys(envConfig.Environments)) | |
| return false, nil | |
| } | |
| } |
Add environment-based variable replacement for stacks
This PR implements environment-specific variable replacement for Docker Swarm stacks, allowing different configurations per environment (dev, staging, prod, etc.) stored directly in the stack's Git repository.
How it works
Environment detection: SwarmCD reads the environment from a Docker Swarm manager node label (default:
swarmcd.environment)Configuration file: Each stack can define an
environments_fileinstacks.yamlpointing to a YAML file in the repository with environment-specific variablesVariable replacement: Variables in the format
${VAR_NAME}or$VAR_NAMEin the compose file are replaced with values from the environments fileStack filtering: Stacks with an
environments_fileare only deployed if:Example
stacks.yaml:
environments.yml (in repository):
docker-compose.yml (in repository):
Benefits
Backward compatibility
environments_filedeploy normallyFiles changed
util/config.go: AddedEnvironmentsFilefield toStackConfigswarmcd/init.go: Added environment detection from Docker Swarm node labelsswarmcd/stack.go: Implemented variable loading and replacement logicswarmcd/stack_test.go: Added comprehensive tests for the new functionalitydocs/ENVIRONMENT_FILTERING.md: Complete documentation with examplesdocs/stacks.yaml: Updated withenvironments_fileexampledocs/environments.yaml: Example environments fileTesting
All tests pass successfully (8/8):