38 messages
Discuss atmos core development (golang). If you want to help out, reach out to @Erik Osterman (Cloud Posse)
Erik Osterman (Cloud Posse)12 months ago
@Andriy Knysh (Cloud Posse) this is ready for final review https://github.com/cloudposse/atmos/pull/1068/files
Erik Osterman (Cloud Posse)12 months ago
@Andriy Knysh (Cloud Posse) this is ready for final review https://github.com/cloudposse/atmos/pull/1102
Erik Osterman (Cloud Posse)12 months ago
@Andriy Knysh (Cloud Posse) this is ready for final review https://github.com/cloudposse/atmos/pull/984
Erik Osterman (Cloud Posse)12 months ago
The golangci-lint supports a language server, so it can be integrated with vscode: https://golangci-lint.run/welcome/integrations/
Erik Osterman (Cloud Posse)12 months ago
@Andriy Knysh (Cloud Posse) this is ready for final review: https://github.com/cloudposse/atmos/pull/1111
let's batch release with other PRs
let's batch release with other PRs
PePe Amengual12 months ago
@Igor Rodionov @Erik Osterman (Cloud Posse) https://github.com/cloudposse/github-action-atmos-affected-stacks/pull/66
Erik Osterman (Cloud Posse)12 months ago
Nightly prereleases should be coming this week. With this, we will start merging PRs with passing tests more aggressively again, and using prereleases to continue to get user feedback earlier while maintaining stability of release
https://github.com/cloudposse/atmos/pull/1114
https://github.com/cloudposse/atmos/pull/1114
S
Shubham Tholiya12 months ago
Getting this error in test cases for: Remove unused flag code in root by samtholiya ยท Pull Request #1103 ยท cloudposse/atmos
Does not look related to my code
cc: @Erik Osterman (Cloud Posse)
Does not look related to my code
cc: @Erik Osterman (Cloud Posse)
Carlos Reyna12 months ago(edited)
Greetings! Quick questions.
Why does docs.go handle the initialization of configurations for the atmos app?
I ask wondering why we do not execute the initialization before calling the command
Why does docs.go handle the initialization of configurations for the atmos app?
info := schema.ConfigAndStacksInfo{
Component: args[0],
ComponentFolderPrefix: "terraform",
}
atmosConfig, err := cfg.InitCliConfig(info, true)
if err != nil {
u.PrintErrorMarkdownAndExit("", err, "")
}I ask wondering why we do not execute the initialization before calling the command
var docsCmd = &cobra.Command{
Use: "docs",
Short: "Open Atmos documentation or display component-specific docs",
Long: `This command opens the Atmos docs or displays the documentation for a specified Atmos component.`,
Example: "atmos docs vpc",
Args: cobra.MaximumNArgs(1),
FParseErrWhitelist: struct{ UnknownFlags bool }{UnknownFlags: false},
ValidArgsFunction: ComponentsArgCompletion,
Run: func(cmd *cobra.Command, args []string) {Carlos Reyna12 months ago
Another question. Looks like there is a new error print command
u.PrintErrorMarkdownAndExit though it was only changed on two lines. Is there a reason we did not use it for other errors? // Construct the full path to the Terraform component by combining the Atmos base path, Terraform base path, and component name
componentPath := filepath.Join(atmosConfig.BasePath, atmosConfig.Components.Terraform.BasePath, info.Component)
componentPathExists, err := u.IsDirectory(componentPath)
if err != nil {
u.PrintErrorMarkdownAndExit("", err, "")
}
if !componentPathExists {
u.PrintErrorMarkdownAndExit("", fmt.Errorf("Component `%s` not found in path: `%s`", info.Component, componentPath), "")
}
readmePath := filepath.Join(componentPath, "README.md")
if _, err := os.Stat(readmePath); err != nil {
if os.IsNotExist(err) {
u.LogErrorAndExit(fmt.Errorf("No README found for component: %s", info.Component))
} else {
u.LogErrorAndExit(fmt.Errorf("Component %s not found", info.Component))
}
}
readmeContent, err := os.ReadFile(readmePath)
if err != nil {
u.LogErrorAndExit(err)
}Erik Osterman (Cloud Posse)12 months ago
Ready for merge or final review. @Andriy Knysh (Cloud Posse) we are going to merge a lot of long standing PRs and release with nightly. Let them burn in and test the changes together
https://github.com/cloudposse/atmos/pull/1093
https://github.com/cloudposse/atmos/pull/1093
Erik Osterman (Cloud Posse)12 months ago
Another one ready for final review, merge and release as part of nightly. https://github.com/cloudposse/atmos/pull/1036
Erik Osterman (Cloud Posse)12 months ago
Let's consider using more
severity: warning rather than changing the limits of lint rules. The lint rules are the target, and once the code quality improves we can increase severity, rather than lowering the bar now.Erik Osterman (Cloud Posse)11 months ago
@Andriy Knysh (Cloud Posse) this is passing tests now and long overdue. Let's merge this and release with nightly RCs.
https://github.com/cloudposse/atmos/pull/1093
https://github.com/cloudposse/atmos/pull/1093
Erik Osterman (Cloud Posse)11 months ago
I am going to merge these tonight, assuming no merge conflicts and tests are passing after rebasing.
โข Fix OCI Tests & Vendoring
โข Implement: atmos list values
โข Fix Vendoring Issues with Globs and Symlinks
โข Convert SCP-style URLs (no explicit scheme) into proper SSH URLs
โข Update dry run for atmos vendor pull to support ssh + detailed SCP urls alignment
โข Enhance Atmos Configuration with atmos.d Support and Import Functionality
And we'll do an RC for it.
โข Fix OCI Tests & Vendoring
โข Implement: atmos list values
โข Fix Vendoring Issues with Globs and Symlinks
โข Convert SCP-style URLs (no explicit scheme) into proper SSH URLs
โข Update dry run for atmos vendor pull to support ssh + detailed SCP urls alignment
โข Enhance Atmos Configuration with atmos.d Support and Import Functionality
And we'll do an RC for it.
Erik Osterman (Cloud Posse)11 months ago
@Andriy Knysh (Cloud Posse) can you confirm this is wrong:
https://github.com/cloudposse/atmos/pull/1036#discussion_r1992528210
https://github.com/cloudposse/atmos/pull/1036#discussion_r1992528210
Shubham Tholiya11 months ago(edited)
@Igor Rodionov
I would like to know if my understanding is correct.
Fix go releaser configs to support pre-releases by goruha ยท Pull Request #1117 ยท cloudposse/atmos
Was this fix done so that the pre-release tag works correctly?
The reason i am asking is that would explain: https://sweetops.slack.com/archives/C031919U8A0/p1741647900276529
I would like to know if my understanding is correct.
Fix go releaser configs to support pre-releases by goruha ยท Pull Request #1117 ยท cloudposse/atmos
Was this fix done so that the pre-release tag works correctly?
The reason i am asking is that would explain: https://sweetops.slack.com/archives/C031919U8A0/p1741647900276529
Erik Osterman (Cloud Posse)11 months ago
@Haitham can you take a look at why this is failing?
https://github.com/cloudposse/atmos/actions/runs/13838449540/job/38719907385?pr=1061
@Andrey Kalmykov merged in main, after we merged in your OCI fixes.
https://github.com/cloudposse/atmos/pull/1061
=== RUN TestExecuteVendorPull
2025/03/13 15:38:31 INFO Vendoring from 'vendor.yaml'
2025/03/13 15:38:31 WARN No TTY detected. Falling back to basic output. This can happen when no terminal is attached or when commands are pipelined.
2025/03/13 15:38:32 ERRO Failed to pull OCI image image=<http://ghcr.io/cloudposse/atmos/tests/fixtures/components/terraform/myapp:v0|ghcr.io/cloudposse/atmos/tests/fixtures/components/terraform/myapp:v0> error="GET <https://ghcr.io/token?scope=repository%3Acloudposse%2Fatmos%2Ftests%2Ffixtures%2Fcomponents%2Fterraform%2Fmyapp%3Apull&service=ghcr.io>: UNAUTHORIZED: authentication required"
2025/03/13 15:38:32 ERRO Failed to vendor myapp1: error : myapp1: failed to process OCI image: failed to pull image: failed to pull image '<http://ghcr.io/cloudposse/atmos/tests/fixtures/components/terraform/myapp:v0|ghcr.io/cloudposse/atmos/tests/fixtures/components/terraform/myapp:v0>': GET <https://ghcr.io/token?scope=repository%3Acloudposse%2Fatmos%2Ftests%2Ffixtures%2Fcomponents%2Fterraform%2Fmyapp%3Apull&service=ghcr.io>: UNAUTHORIZED: authentication requiredhttps://github.com/cloudposse/atmos/actions/runs/13838449540/job/38719907385?pr=1061
@Andrey Kalmykov merged in main, after we merged in your OCI fixes.
https://github.com/cloudposse/atmos/pull/1061
Erik Osterman (Cloud Posse)11 months ago
@Haitham @Shubham Tholiya both of you are modifying atmos config handling.
https://github.com/cloudposse/atmos/pull/1123
https://github.com/cloudposse/atmos/pull/1074
I don't know what the right way to handle it, whose should win. But I am inclined to say @Haitham since it has the most tests, and is the direction we want to move with Viper .
https://github.com/cloudposse/atmos/pull/1123
https://github.com/cloudposse/atmos/pull/1074
I don't know what the right way to handle it, whose should win. But I am inclined to say @Haitham since it has the most tests, and is the direction we want to move with Viper .
Erik Osterman (Cloud Posse)11 months ago
@Andriy Knysh (Cloud Posse) I see @Vinny has a PR with a test failure related to your recent tests I think
=== RUN TestTerraformGenerateBackendCmd
2025/03/14 18:07:20 INFO {
"terraform": {
"backend": {
"s3": {
"acl": "bucket-owner-full-control",
"bucket": "nonprod-tfstate",
"dynamodb_table": "nonprod-tfstate-lock",
"encrypt": true,
"key": "terraform.tfstate",
"region": "us-east-2",
"workspace_key_prefix": "mock"
}
}
}
}
terraform_generate_backend_test.go:49:
Error Trace: /home/runner/work/atmos/atmos/cmd/terraform_generate_backend_test.go:49
Error: "" does not contain "nonprod-tfstate-lock"
Test: TestTerraformGenerateBackendCmd
Messages: 'TestTerraformGenerateBackendCmd' output should contain information about the generated backend
--- FAIL: TestTerraformGenerateBackendCmd (0.02s)Erik Osterman (Cloud Posse)11 months ago
@Andriy Knysh (Cloud Posse) why do we set base path to โโ?
https://raw.githubusercontent.com/cloudposse/atmos/refs/heads/main/atmos.yaml
Cc @Haitham
https://raw.githubusercontent.com/cloudposse/atmos/refs/heads/main/atmos.yaml
Cc @Haitham
V
Vinny11 months ago
@Erik Osterman (Cloud Posse) Looks like the autofix api is down
https://api.github.com/repos/tj-actions/changed-files/tarball/d6e91a2266cdb9d62096cebf1e8546899c6aa18f
https://api.github.com/repos/tj-actions/changed-files/tarball/d6e91a2266cdb9d62096cebf1e8546899c6aa18f
Andrey Kalmykov11 months ago
@Haitham hi, running
Was just wondering, if you had issue like this when you worked on Fix OCI PR? if I try to access this repo using my token, I'm getting access denied, while other public repos are accesible. E.g. like this
make testacc in the main, and getting this error (both in Github flow and locally):=== RUN TestExecuteVendorPull
2025/03/17 20:24:04 INFO Vendoring from 'vendor.yaml'
2025/03/17 20:24:04 WARN No TTY detected. Falling back to basic output. This can happen when no terminal is attached or when commands are pipelined.
2025/03/17 20:24:05 ERRO Failed to pull OCI image image=<http://ghcr.io/cloudposse/atmos/tests/fixtures/components/terraform/myapp:v0|ghcr.io/cloudposse/atmos/tests/fixtures/components/terraform/myapp:v0> error="GET <https://ghcr.io/token?scope=repository%3Acloudposse%2Fatmos%2Ftests%2Ffixtures%2Fcomponents%2Fterraform%2Fmyapp%3Apull&service=ghcr.io>: UNAUTHORIZED: authentication required"
2025/03/17 20:24:05 ERRO Failed to vendor myapp1: error : myapp1: failed to process OCI image: failed to pull image: failed to pull image '<http://ghcr.io/cloudposse/atmos/tests/fixtures/components/terraform/myapp:v0|ghcr.io/cloudposse/atmos/tests/fixtures/components/terraform/myapp:v0>': GET <https://ghcr.io/token?scope=repository%3Acloudposse%2Fatmos%2Ftests%2Ffixtures%2Fcomponents%2Fterraform%2Fmyapp%3Apull&service=ghcr.io>: UNAUTHORIZED: authentication required
2025/03/17 20:24:05 INFO x myapp1 (v0)
2025/03/17 20:25:12 INFO โ github/stargazers (main)
2025/03/17 20:25:12 INFO โ test-components (main)
2025/03/17 20:25:50 INFO โ weather (main)
2025/03/17 20:25:50 INFO โ myapp2
2025/03/17 20:25:50 INFO Vendored 4 components. Failed to vendor 1 components.
2025/03/17 20:25:50 INFO Vendored 5 components.
vendor_utils_test.go:115:
Error Trace: C:/Users/nadia/Desktop/projects/atmos/internal/exec/vendor_utils_test.go:115
Error: Received unexpected error:
failed to vendor components: 1
Test: TestExecuteVendorPull
--- FAIL: TestExecuteVendorPull (106.75s)Was just wondering, if you had issue like this when you worked on Fix OCI PR? if I try to access this repo using my token, I'm getting access denied, while other public repos are accesible. E.g. like this
nadia@ANALITIKASI MINGW64 ~/Desktop/projects/atmos (main)
$ docker pull <http://ghcr.io/cloudposse/atmos/tests/fixtures/components/terraform/myapp:v0|ghcr.io/cloudposse/atmos/tests/fixtures/components/terraform/myapp:v0>
Error response from daemon: denied
nadia@ANALITIKASI MINGW64 ~/Desktop/projects/atmos (main)
$ docker pull <http://ghcr.io/stefanprodan/podinfo:latest|ghcr.io/stefanprodan/podinfo:latest>
latest: Pulling from stefanprodan/podinfo
f18232174bc9: Downloading 1.444MB/3.642MB
a44ca8345dfa: Downloading 567.1kB/2.662MB
4f4fb700ef54: Download completeE
erik11 months ago
@Matt Calhoun could this also maybe be why it's failing on windows? something keychain related.
V
Vinny11 months ago
Are we integrating these labels manually?
Andrey Kalmykov11 months ago
@Andriy Knysh (Cloud Posse) hi, could you please take a look at these ones when you have a minute? The merge order is important, has to be like this
1. PR 984 (globs),
2. PR 1149 (scp ssh)
3. PR 1076 (dry run)
1. PR 984 (globs),
2. PR 1149 (scp ssh)
3. PR 1076 (dry run)
Vinny11 months ago
@Andriy Knysh (Cloud Posse) can you please start review this https://github.com/cloudposse/atmos/pull/1160
@Erik Osterman (Cloud Posse) next step will be
โข Trace PR
โข Styles PR
@Erik Osterman (Cloud Posse) next step will be
โข Trace PR
โข Styles PR
Carlos Reyna11 months ago
Question: I have the following test. If I set log level to debug, I get match errors from the stack trace memory addresses not matching the stderr snapshots. If I leave log level as info, then I cannot test a
log.Debug call is executing. What is the work around to get test the log.Debug message and not error from a stack trace message in stderr snapshot? - name: atmos docs no component 2
enabled: true
snapshot: true
description: "Ensure atmos docs command executes with error component not found."
workdir: "fixtures/scenarios/core/"
command: "atmos"
args:
- "docs"
- "bucket6"
expect:
diff: []
exit_code: 1
stdout:
- "no such file or directory"
stderr:
- "component not found"Erik Osterman (Cloud Posse)11 months ago
@Andriy Knysh (Cloud Posse) also this is ready https://github.com/cloudposse/atmos/pull/1162 (some light refactoring) cc @Vinny
Erik Osterman (Cloud Posse)11 months ago
What are the next steps on https://github.com/cloudposse/atmos/pull/1036? @Vinny do you need @Andriy Knysh (Cloud Posse)โs help to ensure templating works?
Shubham Tholiya11 months ago(edited)
So talking about ticket: DEV-3093 Create a cli command core library
I went into depth of setting config with the help of viper and cobra alone.
There are certain things that i found:
1. Flags are only parsed once root.Execute happens and therefore logs-level, logs-file might have to be manually parsed at the start (Which i don't like)
2. others would be handled as most of them don't need to be initialized before the command.
So i am thinking of handling all other flags based on the common config structure except the logs-level.
Are we fine with this or should i try more to find a way to fix the logs-level issue.
cc: @Erik Osterman (Cloud Posse) @Andriy Knysh (Cloud Posse)
I went into depth of setting config with the help of viper and cobra alone.
There are certain things that i found:
1. Flags are only parsed once root.Execute happens and therefore logs-level, logs-file might have to be manually parsed at the start (Which i don't like)
2. others would be handled as most of them don't need to be initialized before the command.
So i am thinking of handling all other flags based on the common config structure except the logs-level.
Are we fine with this or should i try more to find a way to fix the logs-level issue.
cc: @Erik Osterman (Cloud Posse) @Andriy Knysh (Cloud Posse)
Haitham11 months ago
@Shubham Tholiya
I have a PR open: PR #1091, where I added
I noticed that in
To maintain consistency, we should update
We also need to modify the validation logic to support multiple configuration file paths.
I have a PR open: PR #1091, where I added
--config as a global flag.I noticed that in
validate_editorconfig.go, the validate editorconfig command defines the config flag as a string. However, in my PR, I added the same config flag in root.go as an array of strings.To maintain consistency, we should update
validate_editorconfig.go to handle the config flag as an array of strings. Additionally, the config flag should only be defined in root.go.We also need to modify the validation logic to support multiple configuration file paths.
Erik Osterman (Cloud Posse)11 months ago
@Andriy Knysh (Cloud Posse) https://github.com/cloudposse/atmos/pull/1175/files
Shubham Tholiya11 months ago(edited)
Hey team
Let us make sure that we do not use os.GetEnv or manual flag parsing logic.
Always bind both env and cobra command flag to viper.
I am already migrating existing os.GetEnv logic. So soon we will have a good api that will help us do this efficiently.
Viper will handle the priority
Meanwhile you can use my pr as a reference. It is still in draft. But this will reduce our diversified source of truths for each variables.
@Erik Osterman (Cloud Posse) @Andriy Knysh (Cloud Posse) let us keep this in our reviews ๐
Let us make sure that we do not use os.GetEnv or manual flag parsing logic.
Always bind both env and cobra command flag to viper.
I am already migrating existing os.GetEnv logic. So soon we will have a good api that will help us do this efficiently.
Viper will handle the priority
flag > env > config automatically.Meanwhile you can use my pr as a reference. It is still in draft. But this will reduce our diversified source of truths for each variables.
@Erik Osterman (Cloud Posse) @Andriy Knysh (Cloud Posse) let us keep this in our reviews ๐
Shubham Tholiya11 months ago
Ready for review:
In order:
1. https://github.com/cloudposse/atmos/pull/1146
2. https://github.com/cloudposse/atmos/pull/1147
cc: @Andriy Knysh (Cloud Posse)
In order:
1. https://github.com/cloudposse/atmos/pull/1146
2. https://github.com/cloudposse/atmos/pull/1147
cc: @Andriy Knysh (Cloud Posse)