#atmos-dev (2024-10)

Discuss atmos core development (golang)

2024-10-12

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)
11:56:03 PM

@Erik Osterman (Cloud Posse) has joined the channel

linen avatar
linen
11:56:04 PM

@linen has joined the channel

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
11:56:29 PM

@Andriy Knysh (Cloud Posse) has joined the channel

Igor Rodionov avatar
Igor Rodionov
11:56:29 PM

@Igor Rodionov has joined the channel

Jeremy G (Cloud Posse) avatar
Jeremy G (Cloud Posse)
11:56:29 PM

@Jeremy G (Cloud Posse) has joined the channel

Matt Calhoun avatar
Matt Calhoun
11:56:30 PM

@Matt Calhoun has joined the channel

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)
11:56:55 PM

set the channel topic: Discuss atmos core development (golang)

haitham911eg avatar
haitham911eg
11:57:09 PM

@haitham911eg has joined the channel

Vinny avatar
Vinny
11:57:09 PM

@Vinny has joined the channel

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

@haitham911eg lets move discussion here. @Andriy Knysh (Cloud Posse) can you help answer some questions to unblock him?

1
Sahadat Hossain avatar
Sahadat Hossain
11:58:33 PM

@Sahadat Hossain has joined the channel

haitham911eg avatar
haitham911eg

@Andriy Knysh (Cloud Posse) it is related to the part func LogError log_utils.go line 85

func LogError(cliConfig schema.CliConfiguration, err error)

the cliConfig has value cliConfig.Logs.Level on many cases on call func they pass empty value that force me to add global value branch https://github.com/cloudposse/atmos/tree/DEV-340

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

the LogError functions are called in many diff places

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

eirlier in the pipline (before atmos.yaml is parsed), we don’t have cliConfig yet, so in those cases we use just an empty struct

u.LogError(schema.CliConfiguration{}, err)
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

later in the pipeline, we have already parsed atmos.yaml and have cliConfig in all functions, so we use it

u.LogError(cliConfig, err)
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

(no globals please, they will create a lot of issues)

haitham911eg avatar
haitham911eg

yes I know that globals value causes issues but we have to keep tracking cliConfig.Logs.Level value

haitham911eg avatar
haitham911eg

because we need to handle print logs according to lvl of log

haitham911eg avatar
haitham911eg

I think we should use struct methods instead of normal functions so we can add schema.CliConfiguration{} to struct alternative we pass it on function args

haitham911eg avatar
haitham911eg

on that case no need to global variable

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

we already passing cliConfig as func argument to all functions. In your new code, you can pass cliConfig using any method, including passing as func arg or as struct method

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

Lets keep a list of improvements we can make. But stick with existing conventions so as not to increase the scope.

haitham911eg avatar
haitham911eg

Yes sure we have not increase the scope I just put points to be clear what we should to improve

haitham911eg avatar
haitham911eg

Please @Andriy Knysh (Cloud Posse) review my code on logerror func You will see that I use the args on it

haitham911eg avatar
haitham911eg

Anyway no need to worry about the global value that I added later we can improve But please think on how we can keep tracking log lvl value for all cases

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

@haitham911eg i’ll review your PR

1
kazuki avatar
kazuki
12:06:24 AM

@kazuki has joined the channel

david avatar
david
02:10:47 AM

@david has joined the channel

Michael avatar
Michael
02:51:33 AM

@Michael has joined the channel

2024-10-14

Charles avatar
Charles
05:51:54 PM

@Charles has joined the channel

Sahadat Hossain avatar
Sahadat Hossain

hello! I’m just curious to know whether the examples of quick-start-advanced/vendor.yaml file is synced with latest codebase?

because I was checking this issue: https://github.com/cloudposse/atmos/issues/487 but with current codebase, example and issue they seems mis-synced, in the vendor_component_util.go file where we check the component source uri (it has this checking vendorComponentSpec.Source.Uri ) whereas in the example the source field of the component spec seems an string type! have I misunderstood anything? thank you!

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

@Andriy Knysh (Cloud Posse)

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

vendorComponentSpec.Source.Uri is a string type

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

and we have source: "[github.com/cloudposse/terraform-aws-components.git//modules/vpc?ref={{.Version}}](http://github.com/cloudposse/terraform-aws-components.git//modules/vpc?ref={{.Version}})" in vendor.yaml

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

@Sahadat Hossain please provide more details on your question

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

oh, i see what you are asking

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

those are two diff things

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

we have two types of vendoring in Atmos

Sahadat Hossain avatar
Sahadat Hossain

@Andriy Knysh (Cloud Posse) basically I wanted to check/recreate the issue https://github.com/cloudposse/atmos/issues/487

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
Vendoring | atmos

Use Atmos vendoring to make copies of 3rd-party components, stacks, and other artifacts in your own repo.

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

and these two types of vendoring use diff Go struct in pkg/schema/schema.go

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
#487 Support `file://` paths in `component.yml:spec.source.uri`

Describe the Bug

Specs for component.yml say it supports anything from HashiCorp/go-getter but I am unable to use a local path.

spec: source: uri: ./

kevin@Kevins-Laptop aurora-postgres % atmos vendor pull -c spacelift Pulling sources for the component ‘spacelift’ from ‘file:///Users/kevin/code/sts-devops/components/github-repositories’ into ‘/Users/kevin/code/sts-devops/components/spacelift’

error downloading ‘file:///Users/kevin/code/sts-devops/components/github-repositories’: destination exists and is not a symlink

Expected Behavior

I would like to use a local file path because we are using Atmos features like mixin but we are not using a remotely hosted module/component.

Steps to Reproduce

spec: source: uri: ./

Screenshots

No response

Environment

No response

Additional Context

No response

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

we already support both absolute and relative paths in both component.yaml and vndor.yaml

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

please skip this issue for now, we’ll review it later

Sahadat Hossain avatar
Sahadat Hossain

btw, in this page that you shared both vendoring type referencing to the same page, just letting know incase it may need to update

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
Component Manifest | atmos

Use Component manifests to make copies of 3rd-party components in your own repo.

1
Sahadat Hossain avatar
Sahadat Hossain


please skip this issue for now, we’ll review it later
okay I’ll move to others then, thank you!

melynda.hunter avatar
melynda.hunter
09:19:32 PM

@melynda.hunter has joined the channel

Sahadat Hossain avatar
Sahadat Hossain

@Andriy Knysh (Cloud Posse) for this issue https://linear.app/cloudposse/issue/DEV-2362/atmos-validate-should-not-error-on-empty-files
Running atmos validate on an empty YAML file does not validate
what file it refers by the empty yaml? is this atmos.yaml file or the stacks files? I tried with empty yaml having on the stacks folder’s files but it seems validate with success for those!

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

this is already fixed, the issue can be closed

1
Sahadat Hossain avatar
Sahadat Hossain

I see, okay will make it closed/done (else it will create confusions) and switch to other one!

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

If we run atmos validate on and empty stack directory it should also not error

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

Does that work?

1
Sahadat Hossain avatar
Sahadat Hossain

no for empty stack directory it shows error

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

i explained you in DM how to fix it

Sahadat Hossain avatar
Sahadat Hossain

I’m checking the current imple of the validate_stacks code

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

also please add the check for an empty directory, and don’t show any errors if it’s empty

1
Sahadat Hossain avatar
Sahadat Hossain

in the current codebase it’s hardcodedly using all the paths in the stacks folder but excluding nothing:

	// Include (process and validate) all YAML files in the `stacks` folder in all subfolders
	includedPaths := []string{"**/*"}
	// Don't exclude any YAML files for validation
	excludedPaths := []string{}
	includeStackAbsPaths, err := u.JoinAbsolutePathWithPaths(cliConfig.StacksBaseAbsolutePath, includedPaths)
	if err != nil {
		return err
	}

but in the atmos.yaml it requires to provide stacks.included_paths

if we’re not using those values why to take?

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

please provide more context, that code is from what file and function?

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)


// Don’t exclude any YAML files for validation

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

this means we validate ALL YAML files in stacks

Sahadat Hossain avatar
Sahadat Hossain
	includedPaths := []string{"**/*"}
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

in atmos.yaml, we can exclude the file that are NOT top-level stacks so Atmos does not try to process them as stacks (those can be catalog files, or mixins, or other YAML manifests, but they are not topo-level stacks)

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

in short, we validate ALL YAML files in stacks

Sahadat Hossain avatar
Sahadat Hossain

okay, that means for our problem we just need to check whether the stacks.base_path is empty or not?

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

yes

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

if it’s empty, just show the Atmos logo and exit w/o any errors

1
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

or maybe show a warning message after the logo: “The path pointed to by ‘stacks.base_path’ is empty, there are no stacks defined”

Sahadat Hossain avatar
Sahadat Hossain

yes I thought so, need to show a message that the stack path is empty nothing to validate, like this

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

we already have that ^

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

try to delete/rename the stacks directory and you should have the message above

1
Sahadat Hossain avatar
Sahadat Hossain

thanks for the infos, I’ll make update of this!

2024-10-15

Vinny avatar

hello! hope you guys are good, which person should I ask review in my PR’s?

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

Please tag @Andriy Knysh (Cloud Posse)

1
Vinny avatar

https://linear.app/cloudposse/issue/DEV-2333/atmos-workflows-segfaults cannot reproduce this issue now, I guess its fixed?

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

Yes, sorry about that. I confirmed it’s also working for me in the latest release.

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

We’re meeting later today to groom this backlog.

1
Vinny avatar

no worries, thank you for confirm

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

this was fixed

1
johncblandii avatar
johncblandii
06:21:03 PM

@johncblandii has joined the channel

Michael avatar
Michael

Hey team, I was looking into some Atmos issues and wanted to take a crack at supporting component.yaml|.yml files for vendoring, and I opened a PR here (https://github.com/cloudposse/atmos/pull/725), but I was curious if the team had any preferences or recommendations on how to accomplish it. Feel free to disregard, close, or add another approach and I can take a crack at it

1
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

@Michael thank you, i’ll review the PR today

1
suzuki-shunsuke avatar
suzuki-shunsuke
08:19:59 PM

@suzuki-shunsuke has joined the channel

Cheng JunKai avatar
Cheng JunKai
12:17:22 AM

@Cheng JunKai has joined the channel

2024-10-17

Sahadat Hossain avatar
Sahadat Hossain

@Andriy Knysh (Cloud Posse) hello, would you pls provide give more context for this issue: https://linear.app/cloudposse/issue/DEV-2347/atmos-helmfile-commands-should-not-require-eks? I didn’t fully got the points there!

because I’m seeing the pkg/schema/schema.go file and we’ve only Terraform and Helmfile support in the Component struct, do can’t link with aws thing as this issue is describing!

I assumed few things (clarify if I’m wrong or add further if needed pls): • in the internal/exec/helmfile.go: ◦ we check this if len(cliConfig.Components.Helmfile.ClusterNamePattern) < 1 { condition and if yes then return an error as we’re making this clusterNamePattern required => so according to the issue I assumed this we need to remove to make this field optional, right? • in the internal/exec/aws_eks_update_kubeconfig.go: ◦ in line 182 we’re checking this:

		// `clusterName` can be overridden on the command line
		if clusterName == "" {
			clusterName = cfg.ReplaceContextTokens(context, cliConfig.Components.Helmfile.ClusterNamePattern)
		}
    ▪︎ means now as we want to make this ClusterNamePattern optional so we also need to make sure this is provided before checking this condition, right?

• also as the issue saying the helmfile doesn’t require eks, what does that mean? don’t we gonna keep the UseEKS bool field of helmfile? or clusterNamePatten these things won’t we keep?

Sahadat Hossain avatar
Sahadat Hossain

previously posted in a wrong thread, so posting here again!

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

To reproduce the problem, use this demo

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)
# to run define K3S_TOKEN, K3S_VERSION is optional, eg:
#   K3S_TOKEN=${RANDOM}${RANDOM}${RANDOM} docker-compose up

x-environment: &default-env
  K3S_TOKEN: ${K3S_TOKEN:-test}

services:
  server:
    image: "rancher/k3s:${K3S_VERSION:-latest}"
    command: 
    - 'server'
    tmpfs:
    - /run
    - /var/run
    ulimits:
      nproc: 65535
      nofile:
        soft: 65535
        hard: 65535
    privileged: true
    restart: always
    environment:
      <<: *default-env
      K3S_KUBECONFIG_OUTPUT: /output/kubeconfig.yaml
      K3S_KUBECONFIG_MODE: 666
    volumes:
    - k3s-server:/var/lib/rancher/k3s
    # Output is where the `kubeconfig.yaml` file will be written
    - .:/output
    ports:
    - 6443:6443  # Kubernetes API Server
    - 80:80     # Ingress controller port 80, localhost:8000
    - 443:443   # Ingress controller port 443, localhost:8443
    networks:
      - k3s

  agent:
    image: "rancher/k3s:${K3S_VERSION:-latest}"
    tmpfs:
    - /run
    - /var/run
    ulimits:
      nproc: 65535
      nofile:
        soft: 65535
        hard: 65535
    privileged: true
    restart: always
    environment:
      <<: *default-env
      K3S_URL: <https://server:6443>
    volumes:
    - k3s-agent:/var/lib/rancher/k3s
    networks:
      - k3s
volumes:
  k3s-server: {}
  k3s-agent: {}

networks:
  k3s:
    driver: bridge

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

(which brings up k3s)

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

Then run:

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)
 atmos helmfile apply demo -s dev
Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

It will give an error, because it expects EKS, despite EKS not being a requirement.

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

The current workaround is to do this manually:

atmos helmfile generate varfile demo -s dev
helmfile -f components/helmfile/nginx/helmfile.yaml apply --values dev-demo.helmfile.vars.yaml
Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

The commands I’m referencing are in the workflow file: https://github.com/cloudposse/atmos/blob/main/examples/demo-helmfile/atmos.yaml

base_path: "./"

schemas:
  atmos:
    manifest: "schemas/atmos-manifest.json"

components:
  helmfile:
    base_path: "components/helmfile"
    kubeconfig_path: "{{ .Env.KUBECONFIG }}"
    helm_aws_profile_pattern: default
    cluster_name_pattern: 'demo'

stacks:
  base_path: "stacks"
  included_paths:
    - "**/*"
  excluded_paths:
    - "**/_defaults.yaml"
  name_pattern: "{stage}"

logs:
  file: "/dev/stderr"
  level: Info

# Custom CLI commands

# No arguments or flags are required
commands:
- name: "test"
  description: "Run all tests"
  steps:
  - atmos validate stacks
  # FIXME: `atmos helmfile apply` assumes EKS
  #- atmos helmfile apply demo -s dev
  - atmos helmfile generate varfile demo -s dev
  - helmfile -f components/helmfile/nginx/helmfile.yaml apply --values dev-demo.helmfile.vars.yaml

# Use Nested Custom Commands to provide easier interface for Docker Compose
- name: "k3s"
  commands:
  - name: "up"
    description: Start k3s in the background
    steps:
      - |
        container_running=$(docker ps -q -f "name=k3s")
        if [ -n "$container_running" ]; then
          echo "k3s is already running; run \`atmos k3s down\` to stop it"
        else
          docker compose up -d --wait
        fi

  - name: "down"
    description: Stop k3s
    steps:
      - docker compose down

  - name: "restart"
    description: Restart k3s
    steps:
      - docker compose restart

  - name: "reset"
    description: Reset k3s (delete all data)
    steps:
      - docker compose down --volumes

  - name: "status"
    description: Show the status of k3s
    steps:
      - |
        container_running=$(docker ps -q -f "name=k3s")
        if [ -n "$container_running" ]; then
          docker compose ps --format "{{`{{.Service}} is {{.State}}`}}"
        else
          echo "k3s is not running; run \`atmos k3s up\` to start it"
        fi

- name: "terraform"
  commands:
    - name: "reset"
      description: Delete all local state files
      steps:
        - find . -type f -name "*.tfstate" -delete
        - echo "Deleted all state files"


Sahadat Hossain avatar
Sahadat Hossain

I found that getting this issue is because of the region field was not provided and then after researching find out that in the config we’re by default initializing (in the code) with true value for use_eks flag

Run cd examples/demo-helmfile
  
Invalid endpoint: amazonaws.com
exit status 255
Sahadat Hossain avatar
Sahadat Hossain

so, I’ve update that part

Sahadat Hossain avatar
Sahadat Hossain

@Erik Osterman (Cloud Posse) cc: @Andriy Knysh (Cloud Posse) made the PR, would you pls check? thanks!

https://github.com/cloudposse/atmos/pull/734

#734 Fix issues related to empty stack dir, optional cluster_name_pattern and better of use_eks flag

what

  1. atmos helmfile commands should not require EKS
  2. cluster_name_pattern should only use for EKS
  3. website url link fixing

why

references

1

2024-10-19

Vinny avatar

@Andriy Knysh (Cloud Posse) I guess we need to define the struct of this vendor yaml before hand? if yes can you give a sample please? https://linear.app/cloudposse/issue/DEV-2378/vendor-path-setting-in-atmosyaml-to-specify-vendoryaml-path

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

currently, Atmos searches for vendor.yaml file in the root of the repo

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

we can add a separate section to atmos.yaml

vendor:
  vendor_config_path: <path to vendor.yaml>
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

and add a new struct to pkg/schema/schema.go

Vinny avatar

aham I was think about that

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

to

// CliConfiguration structure represents schema for `atmos.yaml` CLI config
type CliConfiguration struct {
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
// CliConfiguration structure represents schema for `atmos.yaml` CLI config
type CliConfiguration struct {
   Vendor ...
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

then, Atmos should check the path to vendor.yaml from the atmos.yaml first, and use it if it exists

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

if the path is defined in atmos.yaml, but vendor.yaml file is not there, print an error

1
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

if the path is not defined in atmos.yaml, go back to the existing logic - search for vendor.yaml in the root of the repo

1
Vinny avatar

ok so dont need to define in pkg/config/utils.go the vendor struct ? before we load the vendorconfig?

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

just need this

// CliConfiguration structure represents schema for `atmos.yaml` CLI config
type CliConfiguration struct {
   Vendor ...
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

we already have the vendor structure in pkg/schema/schema.go

Vinny avatar

ok thanks for detailed explanation, def makes things faster to me

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
// Atmos vendoring (`vendor.yaml` file)

type AtmosVendorSource struct {
	Component     string   `yaml:"component" json:"component" mapstructure:"component"`
	Source        string   `yaml:"source" json:"source" mapstructure:"source"`
	Version       string   `yaml:"version" json:"version" mapstructure:"version"`
	File          string   `yaml:"file" json:"file" mapstructure:"file"`
	Targets       []string `yaml:"targets" json:"targets" mapstructure:"targets"`
	IncludedPaths []string `yaml:"included_paths,omitempty" json:"included_paths,omitempty" mapstructure:"included_paths"`
	ExcludedPaths []string `yaml:"excluded_paths,omitempty" json:"excluded_paths,omitempty" mapstructure:"excluded_paths"`
	Tags          []string `yaml:"tags" json:"tags" mapstructure:"tags"`
}
1
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
type AtmosVendorConfig struct {
	ApiVersion string `yaml:"apiVersion" json:"apiVersion" mapstructure:"apiVersion"`
	Kind       string `yaml:"kind" json:"kind" mapstructure:"kind"`
	Metadata   AtmosVendorMetadata
	Spec       AtmosVendorSpec `yaml:"spec" json:"spec" mapstructure:"spec"`
}
1
Vinny avatar

what is the best examples folder to test the env variables outputs from terraform?

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

the <https://github.com/cloudposse/atmos/tree/main/examples/tests> folder has all the tests that we execute

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

you can update the base-path here https://github.com/cloudposse/atmos/blob/main/atmos.yaml#L18

base_path: "./examples/tests"
base_path: "./examples/quick-start-advanced"
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

and test any new features. You can also add any new components/stacks for the tests

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

then execute make testacc to run all the tests

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)


env variables outputs from terraform
Don’t understand this relationship between env variables and terraform outputs

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

i guess @Vinny meant how to check that the configured ENV vars are used

2
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

for that, use

ATMOS_LOGS_LEVEL=Trace atmos terraform ....
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

in the output, before the terraform command is executed, you will see

Using ENV variables:
TEST1=test1
TEST2=test2
Vinny avatar

yes just want to see the outputs

2024-10-20

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)
Comment on #731 Added support for remote schemas

@aknysh what do you think about making this the default location for the schema? That way atmos validate always works, even if no schema installed locally.

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

commented

Comment on #731 Added support for remote schemas

@aknysh what do you think about making this the default location for the schema? That way atmos validate always works, even if no schema installed locally.

2024-10-21

2024-10-22

Vinny avatar

Hey, to use a dynamic version in the config.go file, we would need to restructure the version package, as it is currently creating a circular dependency, appreciate any suggestions

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

what do you mean by restructure the version package?

Vinny avatar

creating new pkg/version and importing it back into cmd, that way we can remove the circular dependency

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

ok

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

note that many modules from pkg are used by https://github.com/cloudposse/terraform-provider-utils (it uses the same Atmos code)

cloudposse/terraform-provider-utils

The Cloud Posse Terraform Provider for various utilities (e.g. deep merging, stack configuration management)

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

if you change anything in pkg, please make sure that the provider does not get broken

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

@Vinny let me know if you need access to the terraform-provider-utils repo. We always need to make sure that it compiles

Vinny avatar

ok I have access to that, I’ll check it out thank you

Vinny avatar

https://linear.app/cloudposse/issue/DEV-1880/filter-out-empty-results-from-describe-stacks Do we have any naming requirements for the filter option (e.g., grep or filter), and should it default to filtering out empty results unless explicitly included by a flag? If no filters are applied, what should the default behavior be? Also, can you provide a example folder to test, otherwise I will create one

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

this is the test folder where all the tests go https://github.com/cloudposse/atmos/tree/main/examples/tests

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

it has a lot of diff tests, including for bad/invalid configurations

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

you can add your config and tests for the config

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)


Do we have any naming requirements for the filter option (e.g., grep or filter), and should it default to filtering out empty results unless explicitly included by a flag?
If no filters are applied, what should the default behavior be?

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

let’s just not show the stacks that don’t have any components defined in them

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

w.o using any command-line filters

this1
1

2024-10-23

Charles avatar
Charles
02:13:29 PM

@Charles has joined the channel

2024-10-24

Vinny avatar

my life would change if I had found this earlier

Vinny avatar

@Erik Osterman (Cloud Posse) one question for https://linear.app/cloudposse/issue/DEV-2337/set-custom-user-agent-for-terraform-aws-provider are we going to let users also add an user agent and overwrite the atmos default value or not?

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

Good question, I think user supplied overwrites

1
Vinny avatar

Hey team, The PR #729 for Atmos is ready for review. Please keep the following points in mind:

• The version package has been migrated. This is a bit out of scope for the task, but it was necessary to get the it done. Please review it carefully. I’ve tested everything and ensured it works correctly. I’ve also confirmed there’s no impact on terraform-provider-utils, but I’m not certain if there are other repos that could be affected.

• We might want to inform users about the version package migration.

• I’ve omitted TF_APPEND_USER_AGENT from atmos.yaml since it’s already set by default in the code. However, we might want to add it to all yaml files for better user transparency and guidance on how to use it.

• Rabbit’s comments have been reviewed and replied to, and I’ve closed the remaining ones.

• New test images have been provided in the PR. Please mark addressed comments as resolved, Cheers.

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

thanks @Vinny, I’ll review it

1
Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

Did you update the docs?

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

@Vinny I commented on the PR

Vinny avatar

ok I will cover that !

Vinny avatar

@Andriy Knysh (Cloud Posse) please check if location makes sense to you not sure about that https://github.com/cloudposse/atmos/pull/729/commits/9a7886d1cd9bbdb26347e3914c3ddc7231b16acb

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

I don’t see where we document its usage in atmos.yml

Vinny avatar

atmos.yaml is ignored, see comment in

I've omitted TF_APPEND_USER_AGENT from atmos.yaml since it's already set by default in the code. However, we might want to add it to all yaml files for better user transparency and guidance on how to use it.
Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

Didn’t you originally add it to the atmos.yaml before adding support for the ENV?

Vinny avatar

correct, but I ended up removing because of dynamic version change

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

Note, most things in atmos are controllable via both. The atmos.yaml provides a configurable way, and the ENVs are an alternative.

1
Vinny avatar

but it’s possible to implement

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

But the dynamic version change is just a dynamic default

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

I prefer we keep it how we had it, and preserving the dynamic default

Vinny avatar

ok I will add that, and default for what we had in the code

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)


default for what we had in the code
Default to the dynamic version

Vinny avatar

exactly

1
Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

And update docs

Vinny avatar

@Erik Osterman (Cloud Posse) append_user_agent: "Atmos/<version> (Cloud Posse; +<https://atmos.tools>)"

its good or specify a version? 1.0.0 as example

Vinny avatar

another thing should I add in all examples atmos.yaml or only the root?

1
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

add in root and all the tutorials

1
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

no need to add it to all the tests in examples/tests

1
Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

No, we do not need to add to all examples

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

We only need in documentaiton

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

This is an advanced feature, for a specific use-case. 99% of people do not need to change the default.

2
Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

The reason to expose it in the atmos.yaml is to make disabling it easier. The user can just override it.

1
Vinny avatar

done

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

Close… left a few nit picks

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

Also, I was expecting to see the option for atmos.yaml documented here https://atmos.tools/core-concepts/projects/configuration/terraform

Configure Terraform | atmos

Atmos natively supports opinionated workflows for Terraform and OpenTofu.

Vinny avatar

Ok, I made a big mistake regarding that. In my previous comment, I was referring to the atmos.yaml file, where I intended to omit the variable for that exact purpose.

np1
Vinny avatar

ok I understand the flow now found the related mdx files.

1

2024-10-25

2024-10-26

Vinny avatar

just to clarify here that empty stack means no components right like this example bellow https://linear.app/cloudposse/issue/DEV-1880/filter-out-empty-results-from-describe-stacks

vars:
  stage: dev

import:
  - catalog/myapp

#components:
#  terraform:
#    myapp:
#      vars:
#        location: Stockholm
#        lang: se
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

yes, no components

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

and no imported components (the example above imports one)

1
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

atmos describe stacks - use this as a start

2024-10-28

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

if you are working on a PR and have addressed PR comments, please ping me here for re-review

cc @Vinny @haitham911eg

1
Vinny avatar
#743 Fix example demo-context

what

• Running atmos in demo-context folder causes the code to process all stack configurations, including the catalog stacks

why

• catalogs is a abstract config file, should be excluded to avoid processing that file and output error.

references

#DEV-2691

Summary by CodeRabbit

New Features
• Updated configuration to exclude files and directories matching the path "catalog/**/*" from processing in the stack context.

This change enhances the flexibility of the configuration by allowing users to manage which files are included in the stack operations.

1
1
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

@Vinny did you check that it’s working?

Vinny avatar

yes that works also, in fact is just top level push, same behavior

Vinny avatar
#737 Feat: Vendor path setting in yaml support

what

• Added new support for vendor under atmos yaml • Refactored logic that does check the presence of vendor file

why

• It will allow users to pass new variables that does support the vendor now under atmos config

references

#DEV-2378

CleanShot 2024-10-22 at 23 34 50@2x

Summary by CodeRabbit

New Features
• Introduced a vendor section in the configuration settings, allowing users to specify a path for vendor-specific settings.
• Added support for both absolute and relative paths for the vendor configuration file.
• Added a new command-line flag for specifying the vendor YAML path.
• Enabled dynamic configuration of the vendor YAML path through an environment variable.
• Expanded the CLI configuration to include detailed settings for terraform and helmfile components.
• Introduced aliases for creating shortcuts for CLI commands and enhanced templates settings for advanced templating capabilities. • Bug Fixes
• Enhanced error handling for vendor configuration file retrieval, ensuring clearer messaging when files are not found. • Documentation
• Updated schema to include a new optional Vendor field in the CLI configuration, facilitating vendor-specific configurations.
• Clarified logging behavior and refined paths for validation schemas in the documentation.

haitham911eg avatar
haitham911eg
#740 Do not process stack configs when executing command `atmos vendor pull` and the `--stack` flag is not specified

what

• Do not process stack configs when executing command atmos vendor pull and the --stack flag is not specified • Add AtomsValidateOption to checkAtmosConfig to disable or enable check stack

why

• Atmos vendor should not require stack configs if the stack flag is not provided

references

• DEV-2689 • https://linear.app/cloudposse/issue/DEV-2689/atmos-vendor-should-not-require-stack-configs
image (1)
image (2)

Summary by CodeRabbit

New Features
• Enhanced command functionality to handle the “stack” flag, improving CLI configuration initialization.
• Introduced a new configuration structure for more flexible Atmos configuration checks. • Bug Fixes
• Updated error handling to prevent simultaneous processing of conflicting flags (“component” with “stack” and “component” with “tags”).
• Improved clarity of error messages related to CLI configuration initialization failures.

2024-10-29

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

@Andriy Knysh (Cloud Posse) we don’t use the _yaml_path convention elsewhere, I don’t think we should introduce it here.

Also, since it’s nested underneath vendor, no need to prefix it. It’s also inconsistent with other conventions, see helmfile and terraform, which use base_path cc @Vinny

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)
Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)
#737 Feat: Vendor path setting in yaml support

what

• Added new support for vendor under atmos yaml • Refactored logic that does check the presence of vendor file

why

• It will allow users to pass new variables that does support the vendor now under atmos config

references

#DEV-2378

CleanShot 2024-10-22 at 23 34 50@2x

Summary by CodeRabbit

New Features
• Introduced a vendor section in the configuration settings, allowing users to specify a path for vendor-specific settings.
• Added support for both absolute and relative paths for the vendor configuration file.
• Added a new command-line flag for specifying the vendor YAML path.
• Enabled dynamic configuration of the vendor YAML path through an environment variable.
• Expanded the CLI configuration to include detailed settings for terraform and helmfile components.
• Introduced aliases for creating shortcuts for CLI commands and enhanced templates settings for advanced templating capabilities. • Bug Fixes
• Enhanced error handling for vendor configuration file retrieval, ensuring clearer messaging when files are not found. • Documentation
• Updated documentation to reflect the new vendor.vendor_yaml_path key and other configuration changes.
• Clarified logging behavior and refined paths for validation schemas in the documentation.

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

For helmfile and terraform, it represents a directory. For vendor, we only support a file right now. However, maybe we modify it to support both. If it’s a directory, it processes the directory. If it’s a file, it processes the file.

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

yes, let’s do it

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

ok, @Vinny can you update this so it supports either a file or a directory, and rename vendor_yaml_path to base_path.

Vinny avatar

ok will work on it

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

@Andriy Knysh (Cloud Posse) I think this is ready for merge. You can ignore the failing demo-helmfile test https://github.com/cloudposse/atmos/pull/740

cc @haitham911eg

#740 Do not process stack configs when executing command `atmos vendor pull` and the `--stack` flag is not specified

what

• Do not process stack configs when executing command atmos vendor pull and the --stack flag is not specified • Add AtomsValidateOption to checkAtmosConfig to disable or enable check stack

why

• Atmos vendor should not require stack configs if the stack flag is not provided

references

• DEV-2689 • https://linear.app/cloudposse/issue/DEV-2689/atmos-vendor-should-not-require-stack-configs
image (1)
image (2)

Summary by CodeRabbit

New Features
• Enhanced command functionality to handle the “stack” flag, improving CLI configuration initialization.
• Introduced a new configuration structure for more flexible Atmos configuration checks. • Bug Fixes
• Updated error handling to prevent simultaneous processing of conflicting flags (“component” with “stack” and “component” with “tags”).
• Improved clarity of error messages related to CLI configuration initialization failures.

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

This works for me in my tests in demo-vendoring

#740 Do not process stack configs when executing command `atmos vendor pull` and the `--stack` flag is not specified

what

• Do not process stack configs when executing command atmos vendor pull and the --stack flag is not specified • Add AtomsValidateOption to checkAtmosConfig to disable or enable check stack

why

• Atmos vendor should not require stack configs if the stack flag is not provided

references

• DEV-2689 • https://linear.app/cloudposse/issue/DEV-2689/atmos-vendor-should-not-require-stack-configs
image (1)
image (2)

Summary by CodeRabbit

New Features
• Enhanced command functionality to handle the “stack” flag, improving CLI configuration initialization.
• Introduced a new configuration structure for more flexible Atmos configuration checks. • Bug Fixes
• Updated error handling to prevent simultaneous processing of conflicting flags (“component” with “stack” and “component” with “tags”).
• Improved clarity of error messages related to CLI configuration initialization failures.

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

let me review it

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

before we merge it, @Vinny I think this PR https://github.com/cloudposse/atmos/pull/729 brole the build GH action

#729 Set custom User Agent for Terraform providers

What

• The Atmos command now sets the TF_APPEND_USER_AGENT environment variable, which Terraform uses when interacting with the AWS provider. • Users can also append custom values to TF_APPEND_USER_AGENT, allowing further flexibility in monitoring or tracking specific operations. • The value has been omitted from atmos.yaml as it is already set by default within the code. • If no user-defined value is provided, the system will automatically set atmos {currentVersion} as the default. • A new package, version, has been created and migrated out of the cmd package. This change was necessary to avoid dependency cycles that would have blocked usage in other components. • No dependencies on the version package were identified in the following repository: terraform-provider-utils.

Why

• The current implementation lacks a customer-specific user agent for Terraform operations. This enhancement ensures that Atmos-driven actions are identifiable and distinct from other operations. • Users will be able to differentiate and monitor Atmos-initiated actions within AWS, facilitating better tracking, logging, and troubleshooting.

Proof of working code

New tests created after the requested changes
CleanShot 2024-10-24 at 11 35 21

CleanShot 2024-10-24 at 12 01 49

references

#DEV-2337

Summary by CodeRabbit

Release Notes

New Features
• Introduced --append-user-agent flag for customizing User-Agent strings in Terraform requests.
• Added append_user_agent configuration option in atmos.yaml for user agent specification.
• New commands for backend configuration and variable file generation added to the Atmos CLI.
• Enhanced help documentation and installation instructions for improved user experience.
• Support for --skip-init, --deploy-run-init, and --from-plan flags in Terraform commands. • Bug Fixes
• Improved error handling for command-line arguments related to the new flag. • Documentation
• Updated installation guide and command usage documentation for clarity and new features.

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
#750 installing from brew (linux) showing wrong version (0.0.1) instead latest (1.96.0)

Describe the Bug

After installing atmos via brew (homebrew) on linux it shows outdated version (0.0.1):

❯ brew install atmos
==> Auto-updating Homebrew...
==> Auto-updated Homebrew!
Updated 4 taps (anchore/grype, chainguard-dev/tap, homebrew/core and homebrew/cask).
==> New Formulae
fcft                                   foot                                   node@22

You have 7 outdated formulae installed.

==> Downloading <https://ghcr.io/v2/homebrew/core/atmos/manifests/1.96.0>
############################################################################################################ 100.0%
==> Fetching atmos
==> Downloading <https://ghcr.io/v2/homebrew/core/atmos/blobs/sha256:00eb498b6999c3efa7b3943822968dcd780a097db92cdfe>
############################################################################################################ 100.0%
==> Pouring atmos--1.96.0.x86_64_linux.bottle.tar.gz
==> Caveats
zsh completions have been installed to:
  /home/linuxbrew/.linuxbrew/share/zsh/site-functions
==> Summary
🍺  /home/linuxbrew/.linuxbrew/Cellar/atmos/1.96.0: 10 files, 76.8MB
==> Running `brew cleanup atmos`...

❯ atmos version

 █████  ████████ ███    ███  ██████  ███████ 
██   ██    ██    ████  ████ ██    ██ ██      
███████    ██    ██ ████ ██ ██    ██ ███████ 
██   ██    ██    ██  ██  ██ ██    ██      ██ 
██   ██    ██    ██      ██  ██████  ███████ 
                                             

👽 Atmos 0.0.1 on linux/amd64


Your version of Atmos is out of date. The latest version is 1.96.0

To upgrade Atmos, refer to the following links and documents:

Atmos Releases:
<https://github.com/cloudposse/atmos/releases>

Install Atmos:
<https://atmos.tools/install>

❯ which atmos
/home/linuxbrew/.linuxbrew/bin/atmos

❯ file $(which atmos)
/home/linuxbrew/.linuxbrew/bin/atmos: symbolic link to ../Cellar/atmos/1.96.0/bin/atmos

Maybe wrong version was added to binary?

Expected Behavior

Showing latest version, eg. 1.96.0.

Steps to Reproduce

brew install atmos

Screenshots

No response

Environment

• OS: Ubuntu 22.04 (WSL) • Homebrew: 4.4.3

Additional Context

No response

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

can you please review it

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
      - '-s -w -X "github.com/cloudposse/atmos/pkg/version.Version={{.Version}}"'
Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)
01:58:09 PM
1
Vinny avatar

but 0.0.1 is default version, will be overwrite if you pass the right params during the build process

Vinny avatar

who that works @Andriy Knysh (Cloud Posse)? we need to check that first

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

before, the GH action was working and inserting the corerct version from the release

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

please take a look at it, and I’ll review it as well

1
Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)
    system "go", "build", *std_go_args(ldflags: "-s -w -X 'github.com/cloudposse/atmos/cmd.Version=#{version}'")
Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)
	env GOOS=${GOOS} GOARCH=${GOARCH} go build -o build/atmos -v -ldflags "-X 'github.com/cloudposse/atmos/cmd.Version=${VERSION}'"
Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)
  ldflags = [ "-s" "-w" "-X github.com/cloudposse/atmos/cmd.Version=v${version}" ];
Vinny avatar

checking that

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

We need to update all these other repos

Vinny avatar

of course I forgot to update the brew

Vinny avatar

what’s the process for homebrew? create a fork and pray for they approve fast?

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

i think @RB did it before

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

if you need changes in their repo, fork it and open a PR

Vinny avatar

yes looks like best option now then we have to wait I will explain urgency

Vinny avatar

it works after uninstall my atmos and recreate it using my formulae

Vinny avatar

sending PR

Vinny avatar
#195962 Hotfix atmos: update pkg version

Update breaking change in atmos pkg version

• Have you followed the guidelines for contributing? • Have you ensured that your commits follow the commit style guide? • Have you checked that there aren’t other open pull requests for the same formula update/change? • Have you built your formula locally with HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where <formula> is the name of the formula you’re submitting? • Is your test running fine brew test <formula>, where <formula> is the name of the formula you’re submitting? • Does your build pass brew audit --strict <formula> (after doing HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?


Vinny avatar

@Andriy Knysh (Cloud Posse) please grant write access https://github.com/cloudposse/test-helpers

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

@Vinny grand write access to you?

1
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

granted you write access to the repo, please accept the invitation

Vinny avatar

sorry the typo, thx

Vinny avatar

https://github.com/Homebrew/homebrew-core/pull/195962 @Andriy Knysh (Cloud Posse) PR building

#195962 atmos: update pkg version

Update breaking change in atmos pkg version

• Have you followed the guidelines for contributing? • Have you ensured that your commits follow the commit style guide? • Have you checked that there aren’t other open pull requests for the same formula update/change? • Have you built your formula locally with HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where <formula> is the name of the formula you’re submitting? • Is your test running fine brew test <formula>, where <formula> is the name of the formula you’re submitting? • Does your build pass brew audit --strict <formula> (after doing HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?


RB avatar
RB
05:32:48 PM

@RB has joined the channel

Vinny avatar

https://github.com/cloudposse/test-helpers/pull/31 @Andriy Knysh (Cloud Posse) for review please

#31 fix atmos pkg version

what

• Fix breaking change in atmos version

why

• version pkg has been moved under version folder

references

1
Vinny avatar
1
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

does it mean we can cut new releases now?

Vinny avatar

I guess we can use revisions to update formula, build instructions, docs

Pulak Kanti Bhowmick avatar
Pulak Kanti Bhowmick
04:28:06 AM

@Pulak Kanti Bhowmick has joined the channel

2024-10-30

Vitalii avatar
Vitalii
01:57:18 PM

@Vitalii has joined the channel

Pulak Kanti Bhowmick avatar
Pulak Kanti Bhowmick

Hello Team, I need one clarification on this Default value for demo argument should not cause an error

Where there is multiple arguments are required, if user doesn’t provide one, in that case, it is hard to identify the order of argument given by user. What should we do in this case?

My opinion: For flags, we have that option to keep flag optional with default value. It seems, arguments are always required and users have to pass those. But if user doesn’t give any arguments, only then we can use the default arguments for any command. But If they pass some and skip some, there is no way to identify which arguments are missing and which are given.

Please share your thoughts! Thank you so much!

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

Cobra does all of that and shows the error message and usage if any argument is missing, or a required flag is not provided

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

looks like you are talking abou Atmos custom commands

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
#752 improve custom command error message for missing arguments

what

• improved custom command error message for missing arguments including the name of argument for better user’s understanding.

why

• If a custom command expects an argument, it should say so with the arguments name.

Current:
image

After fix:
image

references

DEV-2324

Summary by CodeRabbit

Summary by CodeRabbit

New Features
• Enhanced error messages for invalid argument counts in command processing, providing clearer guidance on required arguments. • Bug Fixes
• Improved error handling for empty argument names, ensuring better logging and visibility of issues.

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

in custom commands, we have the required feild for flags

type CommandFlag struct {
	Name        string `yaml:"name" json:"name" mapstructure:"name"`
	Shorthand   string `yaml:"shorthand" json:"shorthand" mapstructure:"shorthand"`
	Type        string `yaml:"type" json:"type" mapstructure:"type"`
	Description string `yaml:"description" json:"description" mapstructure:"description"`
	Usage       string `yaml:"usage" json:"usage" mapstructure:"usage"`
	Required    bool   `yaml:"required" json:"required" mapstructure:"required"`
}
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

we can add the same for the arguments here

type CommandArgument struct {
	Name        string `yaml:"name" json:"name" mapstructure:"name"`
	Description string `yaml:"description" json:"description" mapstructure:"description"`
}
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

and add default field to both arguments and flags

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
type CommandFlag struct {
	Name        string `yaml:"name" json:"name" mapstructure:"name"`
	Shorthand   string `yaml:"shorthand" json:"shorthand" mapstructure:"shorthand"`
	Type        string `yaml:"type" json:"type" mapstructure:"type"`
	Description string `yaml:"description" json:"description" mapstructure:"description"`
	Usage       string `yaml:"usage" json:"usage" mapstructure:"usage"`
	Required    bool   `yaml:"required" json:"required" mapstructure:"required"`
    Default     any   `yaml:"default" json:"default" mapstructure:"default"`
}
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
type CommandArgument struct {
	Name        string `yaml:"name" json:"name" mapstructure:"name"`
	Description string `yaml:"description" json:"description" mapstructure:"description"`
	Required    bool   `yaml:"required" json:"required" mapstructure:"required"`
    Default     any   `yaml:"default" json:"default" mapstructure:"default"`
}
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

this way, the user would control what is required and provide the default values

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

the default and required need to be sent to Cobra (as we do for the other fields of the commands), and Cobra will take care of missing arguments and flags and show the message and the usage

Pulak Kanti Bhowmick avatar
Pulak Kanti Bhowmick

thank you for the info. Let me try this approach

1
Pulak Kanti Bhowmick avatar
Pulak Kanti Bhowmick

Hi @Andriy Knysh (Cloud Posse)

I have tried to implement in this way. But while working on this, one more question come to my mind: let’s assume for one custom command, we have two args and user provide only one arg. In this case how we can figure out the arg order i.e. how to determind the user arg is the first arg or the second one?

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

thank you @Pulak Kanti Bhowmick let me review the PR

2024-10-31

Vinny avatar

@Andriy Knysh (Cloud Posse) this is good to review please https://github.com/cloudposse/atmos/pull/737

1
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

@Vinny is the PR ready for review? I know it was ready a few days ago, but maybe you got comments that need to be addressed

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

I don’t see any unresolved comments

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

(or new commits since I said LGTM)

Vinny avatar

will check in few min

    keyboard_arrow_up