Skip to content

feat: introduce conversions between API versions - #4823

Open
AustinAbro321 wants to merge 38 commits into
mainfrom
introduce-api-conversions
Open

feat: introduce conversions between API versions#4823
AustinAbro321 wants to merge 38 commits into
mainfrom
introduce-api-conversions

Conversation

@AustinAbro321

@AustinAbro321 AustinAbro321 commented Apr 13, 2026

Copy link
Copy Markdown
Member

Description

I had previously discussed the possibility of moving the deprecated types into their internal package, but I decided against this as I believe we want SDK users to be able to fully inspect the contents of their package. While variables are set for instance, we should let SDK users examine them.

relates to #3433

Checklist before merging

@codecov

codecov Bot commented Apr 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.52991% with 96 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/cmd/dev_upgrade.go 66.66% 47 Missing and 1 partial ⚠️
src/internal/api/v1alpha1/convert.go 96.85% 20 Missing and 6 partials ⚠️
src/internal/api/v1beta1/convert.go 97.53% 12 Missing and 6 partials ⚠️
src/test/testutil/fuzz.go 90.00% 4 Missing ⚠️
Files with missing lines Coverage Δ
src/api/convert/convert.go 100.00% <100.00%> (ø)
src/api/v1alpha1/package.go 95.91% <100.00%> (+8.96%) ⬆️
src/api/v1beta1/component.go 81.81% <ø> (+38.96%) ⬆️
src/api/v1beta1/package.go 100.00% <100.00%> (+35.71%) ⬆️
src/cmd/dev.go 38.79% <100.00%> (+0.07%) ⬆️
src/test/testutil/fuzz.go 90.00% <90.00%> (ø)
src/internal/api/v1beta1/convert.go 97.53% <97.53%> (ø)
src/internal/api/v1alpha1/convert.go 96.85% <96.85%> (ø)
src/cmd/dev_upgrade.go 66.66% <66.66%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/api/convert/convert.go Outdated
Comment thread src/internal/api/v1beta1/convert.go
Comment thread src/api/convert/convert_test.go
Base automatically changed from update-v1beta1-schema to main July 1, 2026 14:13
@netlify

netlify Bot commented Jul 1, 2026

Copy link
Copy Markdown

Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit 99def50
🔍 Latest deploy log https://app.netlify.com/projects/zarf-docs/deploys/6a67644ab23b290008a6acab

Signed-off-by: Austin Abro <austinabro321@gmail.com>
@AustinAbro321
AustinAbro321 force-pushed the introduce-api-conversions branch from cfef0e1 to 54d2861 Compare July 1, 2026 16:43
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
@AustinAbro321
AustinAbro321 marked this pull request as ready for review July 1, 2026 18:57
@AustinAbro321
AustinAbro321 requested review from a team as code owners July 1, 2026 18:57
AustinAbro321 and others added 9 commits July 1, 2026 15:11
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>

@soltysh soltysh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A few comments/questions, the reflection-based round tripping tests should help us uncover eventual missing bits. Those few I've pointed out are b/c AI told me 😅

Comment thread src/internal/api/v1alpha1/convert.go Outdated
Comment thread src/internal/api/v1alpha1/convert.go Outdated
Comment thread src/api/v1beta1/convert.go Outdated
Comment thread src/internal/api/v1alpha1/roundtrip_test.go
Name: c.Name,
Description: c.Description,
Optional: c.Optional,
Service: string(c.Service),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here, the opposite, so Required: !c.Optional, to ensure that the generic struct has all the fields filled in.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I have not filled in every field in the generic struct, but rather let the conversion logic handle it so long as there is no information missing by filling out the newer, canonical field. In this case since required would always be nil, the conversion logic fallsback to using optional.

Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
@AustinAbro321
AustinAbro321 requested a review from soltysh July 15, 2026 14:21
AustinAbro321 and others added 11 commits July 20, 2026 11:28
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>

@brandtkeller brandtkeller left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Adding some thoughts as I get time to test.

Group: c.DeprecatedGroup,
DataInjections: dataInjectionsToGeneric(c.DataInjections),
HealthChecks: healthChecksToGeneric(c.HealthChecks),
DeprecatedScripts: scriptsToGeneric(c.DeprecatedScripts),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are we consuming DeprecatedScripts anywhere to perform the migration to actions?

@AustinAbro321 AustinAbro321 Jul 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes in the pkgcfg package, It should always be migrated right as v1alpha1 is parsed, and so technically not be needed in the migration logic, but I think it's still better to have the converter convert everything.

func actionSetToGeneric(s v1alpha1.ZarfComponentActionSet) types.ComponentActionSet {
defaults := types.ComponentActionDefaults{
Silent: s.Defaults.Mute,
MaxTotalSeconds: int32(s.Defaults.MaxTotalSeconds),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

seems negligible but do we need to add safe migrations from int to int32?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not concerned about that honestly

Comment thread src/api/v1beta1/component.go Outdated
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants