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.

Sign up for GitHub

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

Jump to bottom

feat: set variables on find images #2282

Merged
merged 87 commits into from
Mar 7, 2024
Merged

feat: set variables on find images #2282

merged 87 commits into from
Mar 7, 2024

Conversation

AustinAbro321
Copy link
Contributor

@AustinAbro321 AustinAbro321 commented Feb 1, 2024 •
edited by Noxsios
Loading

Description

Currently when you run zarf prepare find-images with a package that has variables in it, the variables don't get set, even to their defaults. This can break the helm chart, or make us unable to find images. This PR introduces a --deploy-set flag. Additionally we take in the default values to the chart.

Type of change

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

Checklist before merging

@netlify Netlify
Copy link

netlify bot commented Feb 1, 2024 •
edited
Loading

✅ Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit e4d4543
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/65e9cbceecda760008c87853

@AustinAbro321 AustinAbro321 marked this pull request as ready for review February 5, 2024 21:16
Racer159
Racer159 requested changes Feb 5, 2024
View reviewed changes
src/cmd/dev.go Outdated Show resolved Hide resolved
src/config/lang/english.go Outdated Show resolved Hide resolved
src/internal/packager/helm/chart.go Outdated Show resolved Hide resolved
src/internal/packager/helm/common.go Show resolved Hide resolved
src/internal/packager/helm/common.go Show resolved Hide resolved
src/types/runtime.go Outdated Show resolved Hide resolved
src/internal/packager/helm/prepare-render.go Outdated Show resolved Hide resolved
src/test/packages/00-find-images/zarf.yaml Outdated Show resolved Hide resolved
src/test/e2e/00_use_cli_test.go Outdated Show resolved Hide resolved
eddiezane
eddiezane reviewed Mar 5, 2024
View reviewed changes
src/test/common.go Outdated Show resolved Hide resolved
lucasrod16
lucasrod16 previously approved these changes Mar 6, 2024
View reviewed changes
Copy link
Contributor

@lucasrod16 lucasrod16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@AustinAbro321 AustinAbro321 mentioned this pull request Mar 6, 2024
Open
Noxsios
Noxsios suggested changes Mar 6, 2024
View reviewed changes
Copy link
Contributor

@Noxsios Noxsios left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one final tweak (my fault), then its gtg

src/pkg/packager/prepare.go Outdated Show resolved Hide resolved
Racer159
Racer159 previously approved these changes Mar 6, 2024
View reviewed changes
Copy link
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approving to dismiss req'd changes, agree with @Noxsios on pointers

lucasrod16
lucasrod16 approved these changes Mar 7, 2024
View reviewed changes
Copy link
Contributor

@lucasrod16 lucasrod16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Noxsios Noxsios merged commit 9b3dfb6 into main Mar 7, 2024
29 checks passed
@Noxsios Noxsios deleted the set-variables-on-find-images branch March 7, 2024 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Racer159 Racer159 Racer159 left review comments

@eddiezane eddiezane eddiezane left review comments

@lucasrod16 lucasrod16 lucasrod16 approved these changes

@Noxsios Noxsios Noxsios approved these changes

@jeff-mccoy jeff-mccoy Awaiting requested review from jeff-mccoy

@Madeline-UX Madeline-UX Awaiting requested review from Madeline-UX

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@AustinAbro321 @eddiezane @Racer159 @Noxsios @lucasrod16