Skip to content
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

Refactor actions to move to common library #2253

Closed
AustinAbro321 opened this issue Jan 24, 2024 · 4 comments
Closed

Refactor actions to move to common library #2253

AustinAbro321 opened this issue Jan 24, 2024 · 4 comments
Assignees

Comments

@AustinAbro321
Copy link
Contributor

Describe what should be investigated or refactored

The actions functionality of zarf should be refactored and moved to https://github.com/defenseunicorns/pkg so it can by other projects.

@Racer159
Copy link
Contributor

Tying this to this: #2203 since they are related

@Racer159
Copy link
Contributor

We should ensure we add unit tests during this migration: #1750

@zack-is-cool
Copy link
Contributor

zack-is-cool commented Jan 30, 2024

requesting an easier pattern to set ZARF_VARs

Currently now there's no way to set multiple ZARF_VARs that have different values in a single action.
Something like how github actions implemented scales nicely

For example:

components:
  - name: fetch-staged-aws-ssm-vars
    required: false
    description: "Fetch staged json object from AWS SSM Parameter Store and extract values for zarf variables"
    actions:
      onDeploy:
        before:
          # get the cluster name
          - cmd: kubectl config current-context | awk -F'[:/]' '{print $NF}'
            setVariables:
              - name: CLUSTER_NAME
          - cmd: aws eks describe-cluster --name ${ZARF_VAR_CLUSTER_NAME} --query 'cluster.endpoint' --output text | cut -d . -f3
            setVariables:
              - name: CLUSTER_AWS_REGION
          - cmd: aws ssm get-parameter --name "/${ZARF_VAR_CLUSTER_NAME}/${ZARF_VAR_CLUSTER_AUTOSCALER_HELM_INPUT_VALUES_PATH}" --with-decryption
            setVariables:
              - name: CLUSTER_AUTOSCALER_HELM_INPUT_VALUES
          - cmd: jq -r '.Parameter.Value | fromjson | .iam_role_arn' <<< "$ZARF_VAR_CLUSTER_AUTOSCALER_HELM_INPUT_VALUES"
            setVariables:
              - name: IAM_ROLE_ARN
          - cmd: jq -r '.Parameter.Value | fromjson | .service_account' <<< "$ZARF_VAR_CLUSTER_AUTOSCALER_HELM_INPUT_VALUES"
            setVariables:
              - name: SERVICE_ACCOUNT

could become

  - name: fetch-staged-aws-ssm-vars
    required: false
    description: "Fetch staged json object from AWS SSM Parameter Store and extract values for zarf variables"
    actions:
      onDeploy:
        before:
          - cmd: |
              echo "CLUSTER_NAME=$(kubectl config current-context | awk -F'[:/]' '{print $NF}')" >> ZARF_ENV
              echo "CLUSTER_AWS_REGION=$(aws eks describe-cluster --name ${ZARF_VAR_CLUSTER_NAME} --query 'cluster.endpoint' --output text | cut -d . -f3)" >> ZARF_ENV
              echo "CLUSTER_AUTOSCALER_HELM_INPUT_VALUES=$(aws ssm get-parameter --name "/${ZARF_VAR_CLUSTER_NAME}/${ZARF_VAR_CLUSTER_AUTOSCALER_HELM_INPUT_VALUES_PATH}" --with-decryption)" >> ZARF_ENV
              echo "IAM_ROLE_ARN=$(jq -r '.Parameter.Value | fromjson | .iam_role_arn' <<< "$ZARF_VAR_CLUSTER_AUTOSCALER_HELM_INPUT_VALUES")" >> ZARF_ENV
              echo "SERVICE_ACCOUNT=$(jq -r '.Parameter.Value | fromjson | .service_account' <<< "$ZARF_VAR_CLUSTER_AUTOSCALER_HELM_INPUT_VALUES")" >> ZARF_ENV

@Racer159 Racer159 added this to the (2024.02.20) milestone Feb 2, 2024
@eddiezane eddiezane moved this to In progress in Zarf (old) Mar 4, 2024
Noxsios added a commit that referenced this issue Apr 22, 2024
## Description

Part of the actions refactor (split to reduce review time).

This PR splits variables into it's own library that does not depend on
Zarf specific code, fixes a bug with the new Helm Chart variables
injection and overhauls the variables docs to be more readable and
understandable and to include information on internal variables and the
Helm Chart variables.

## Related Issue

Relates to #2253
Fixes #2395
Fixes #1374

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Other (security config, docs update, etc)

## Checklist before merging

- [X] Test, docs, adr added or updated as needed
- [X] [Contributor Guide
Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow)
followed

---------

Co-authored-by: Lucas Rodriguez <[email protected]>
Co-authored-by: Austin Abro <[email protected]>
Co-authored-by: razzle <[email protected]>
@salaxander salaxander removed this from the (2024.02.20) milestone Jul 2, 2024
@salaxander salaxander added this to Zarf Jul 22, 2024
@github-project-automation github-project-automation bot moved this to Backlog in Zarf Jul 22, 2024
@AustinAbro321
Copy link
Contributor Author

We are no longer going to move actions to a common library, but I like the suggestion that you have here @zack-is-cool, would you mind creating a separate issue for your suggestion on variables. Even if we don't do implement your suggestion, I do think we can find ways to improve how we set variables in cmd

@github-project-automation github-project-automation bot moved this from Backlog to Done in Zarf Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

6 participants