Skip to content

Include ytt overlays in kctrl package install dry-run output - #1877

Open
NopeCharon wants to merge 1 commit into
carvel-dev:developfrom
NopeCharon:fix-pkgi-dry-run-overlays
Open

NopeCharon wants to merge 1 commit into
carvel-dev:developfrom
NopeCharon:fix-pkgi-dry-run-overlays

Conversation

@NopeCharon

Copy link
Copy Markdown

What this PR does / why we need it:

kctrl package install --dry-run returns before --ytt-overlay-file is processed, so the printed resources silently omit the overlays Secret and the PackageInstall annotation that references it. Piping the output into kapp/ytt therefore produces an installation without the requested overlays.

With this change, when overlay files are supplied (and --ytt-overlays is not set to false), the dry-run:

  • prints the same <name>-<namespace>-overlays Secret that a regular install creates (built via the existing YttOverlays.OverlaysSecret()), with namespace set like the other printed resources
  • adds the ext.packaging.carvel.dev/ytt-paths-from-secret-name.kctrl-ytt-overlays annotation to the PackageInstall

Like the values Secret in the dry-run output, overlay contents are rendered under stringData so the output stays readable.

Which issue(s) this PR fixes:

Fixes #1389

Does this PR introduce a user-facing change?

`kctrl package install --dry-run` now includes the ytt overlays Secret and the corresponding PackageInstall annotation when `--ytt-overlay-file` is supplied.

Additional Notes for your reviewer:

  • Extended TestPackageInstallDryRun (cli e2e) with a section that passes an overlay via --ytt-overlay-file -. The section fails without the fix and passes with it. The dry-run e2e tests do not talk to a cluster, so I ran them locally with KCTRL_E2E_NAMESPACE=kctrl-test.
  • Also checked by hand: a single file, a directory of overlays, --ytt-overlays=false (no overlays printed), a missing file (returns an error), and that --dry-run --tty=false | ytt -f - parses the output.
Review Checklist:
  • Follows the developer guidelines
  • Relevant tests are added or updated
  • Relevant docs in this repo added or updated
  • Relevant carvel.dev docs added or updated in a separate PR and there's
    a link to that PR
  • Code is at least as readable and maintainable as it was before this
    change

Additional documentation e.g., Proposal, usage docs, etc.:


`kctrl package install --dry-run` returned before the `--ytt-overlay-file`
flag was processed, so the printed resources silently omitted the overlays
Secret and the PackageInstall annotation referencing it. Piping the output
into kapp or ytt therefore produced an installation without the requested
overlays.

When overlay files are supplied (and `--ytt-overlays` is not disabled),
build the same overlays Secret that a regular install creates and print
it along with the other resources, and add the
`ext.packaging.carvel.dev/ytt-paths-from-secret-name.kctrl-ytt-overlays`
annotation to the PackageInstall. As with the values Secret, overlay
contents are rendered under stringData so the output stays readable.

Extend the package install dry-run e2e test to cover overlays supplied
via `--ytt-overlay-file`.

Signed-off-by: NopeCharon <115794254+NopeCharon@users.noreply.github.com>

This branch has not been deployed

No deployments
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.

kctrl package install … --dry-run --ytt-overlay-file … does not stamp out overlays

2 participants