fysa using the latest rc and DAG it seems to break...
# atmos-dev
z
fysa using the latest rc and DAG it seems to break carriage returns in the terminal when using atmos and after the tool has ran @Erik Osterman (Cloud Posse), atleast with my zsh - going to investigate
Pulled upstream origin/main; this checkout is current at e299b5104.
Yes—there’s an Atmos bug in concurrent streaming: pkg/io/line_prefix_writer.go:109 preserves raw \r while injecting
prefixes, and the Terraform adapter uses separate stdout/stderr locks (pkg/scheduler/adapters/terraform.go:1291). That
explains the overwritten/shifted fragments.
Immediate workaround, retaining concurrency:
atmos terraform apply ... --max-concurrency 3 --log-order grouped -auto-approve
That avoids live prefixed streams. No code changed.
issue and pr incoming
e
My concern is Atmos is the one probably generating those lines
And when run in parallel, Atmos should not use spinners
I didn't look at the fix but simply stripping the carriage return is correct when the underlying tool generates it but if Atmos is the one generating it then Atmos should just be smart enough not to emit them to begin with
We recently merged a separate PR that fixed a lot of the TTY handling and detection
My guess is it was broken before. Now we fixed it but now because it's fixed we have this output issue
🤔 1
z
Hmm. I can keep pushing on it unless you wanna take it over
e
I'll prompt coderabbit if my theory stands
Could use the help though, lots of PR contributions lately 😃
👍 1
z
This is the other thing I've been noticing
Copy code
in stack "<MASKED>-dev1".
I know something happened recently with masking, but not exactly sure how it's being triggered lol
e
Because somewhere you have that value as a secret.
z
What makes something a secret?
I don't think I am doing anything fancy in this deployment, just iterating through components in 3 different stacks. The string it's referencing that is being masked is printed everywhere normally before it hits that point
e
Do you use atmos stores?
z
Yeah I think we are using an SSM store
e
Is that store marked secret?
z
Maybe the TF output is sensitive ?
We aren't explicitly marking a store as secret, it's an aws-ssm-parameter-store
e
does that
<MASKED>
value appear in any
variable
input that is also marked sensitive?
(Basically here's how it works: Atmos maintains an in-memory list of every secret recorded. We only know it's a secret if a variable marks it a secret, if the store is marked as secret, or I think if the output is also marked as sensitive. Then any time Atmos sees any one of those strings appear anywhere in the output, we automatically mask it.)
There's no way to know that it's not actually a secret and display it, if it orignated from a secret store. You can disable masking altogether. But somewhere that string was registered as a secret.
Looks like your fix is mostly correct
THere's a nitpick about adding one more test
👍 1
The tests verify
SuppressSpinners()
and its nested cleanup behavior, but they do not verify that
ExecuteTerraform
activates suppression when
MaxConcurrency > 1
and restores it after completion. I recommend adding that adapter-level regression test. It will protect the policy that parallel execution must downgrade transient UI.
Mind adding that?
z
Will do when I hop on 🫡
👍 1
Added
Any leads on this?
z
🤔 I have not configured anything related to the new feature. It's masking part of my stack name. I think it might be related to this
Copy code
hooks:
        store-outputs:
          outputs:
            rds_properties: .rds_properties
That being said, that output isn't actually marked as
sensitive
from the module
it feels random..?
would any
!terraform.state
lookups somehow get marked as sensitive
without explicitly telling atmos they are sensitive
( and none of them are token looking strings )
e
I think it's going to be hard to test this on our end to see where it came from. You have the best visibility into what that MAST value is and where it could have originated from
z
Copy code
ce_port" = 5432
               [justastring-dev1/uds-rds]   "db_name" = "justastring"
                                                         [justastring-dev1/uds-rds] }
                                                                               [justastring-dev1/uds-rds] rds_sensitive_properties = <sensitive>
        INFO  Running hooks event=after.terraform.apply status=success component=uds-rds stack=justastring-dev1
▶ Auto-provisioned JIT workdir for component 'uds-rds' in stack 'justastring-dev1'
                                                                            💡 Tip: use !terraform.state instead of !terraform.output to read outputs without terraform init
✓ Fetching rds_properties output from uds-rds in <MASKED>-dev1
⚠ 3 component(s) skipped after an earlier failure

                                                    Error

                                                    Error: terraform execution failed: component=$platform-uds-pkg-dbeaver stack=<MASKED>-dev1: subcommand exited with code 1

                                                       terraform output:
ignore the error, I didn't have an SSM tunnel to my eks cluster, expected
rds_sensitive_properties = <sensitive>
is a real output from this component (module), and is senstive, but we aren't even consuming that
in the example I'm giving you
justastring
would be the string being
<MASKED>
we're storing
Copy code
hooks:
        store-outputs:
          outputs:
            rds_properties: .rds_properties
but this is a different variable, and not sensitive
also btw this is me running rc5 with the carriage return bug 😂
I think atmos might be masking all strings matching from a component being written to an aws ssm parameter store if any outputs are sensitive
Atleast that's the only thing that would make sense in my scenario
If you see that
"db_name" = "justastring"
- that exists inside the output
rds_properties
¯\_(ツ)_/¯
Anyway, going back to this PR the pipeline failed due to some network connectivity issues it seems github.com/cloudposse/atmos/pull/2860 🙏
justastring
also exists inside of
rds_sensitive_properties
Copy code
output "rds_sensitive_properties" {
  sensitive   = true
  description = <<-EOF
  RDS Sensitive Properties:

    db_instance_username: "The master username for the database"
    db_instance_password: "The password for the master user (null if AWS-managed)"
  EOF
  value = {
    db_instance_username = var.rds_requirements.username
    db_instance_password = var.rds_options.manage_master_user_password ? null : random_password.rds_password[0].result
  }
}
In this case it would be the
db_instance_username
Again, still not actually using that output anywhere, but the string would match but this seems onto something
Copy code
The Recommended Next Step
Check every output on the component, not only rds_properties, for the sensitive = true marker. Run this command:

atmos terraform output <component> -s <stack> --json | jq 'to_entries[] | select(.value.sensitive)'
Or, search the module file <http://outputs.tf|outputs.tf> for the text sensitive = true.

If a sensitive output's value shares text with the stack name, that output is the source of the masked text. The store, the variable, and rds_properties are not the source.
-- looks like I missed a coderabbit request 😨
k - rabbit is green once again
e
z
After approving rabbit said there's an issue 💀
addressing now
e
you jinxed it
😓 1
z
coderabbit won't stop rofl
e
It's very passive aggressive. If it's having a bad day, it just likes to mess with you.
z
Will it ever stop? Or will it seriously nit-pick forever
e
Yeah usually just ultimately stamps it. I haven't seen it be this chatty
are you using codex or claude
z
codex
Unfortunately we aren't able to use claude anymore 🦧
Alright, guess that's better than 10 more cycles of one more gap
shared a GIF
e
@Andriy Knysh (Cloud Posse) I think we need to wait for this to merge too before cutting a release
Since it fixes an output issue when running parallel processes
z
hurry kick the pipeline off before it comes back
✔️ 1
Pushed some fixes for failing checks
✔️ 1
e
@Andriy Knysh (Cloud Posse) this is passing
👀 1
merged 1
I think this is the last PR we needed before cutting a release.