Skip to content

fix: return a valid canonical reproducible form from helm_import_repository#285

Merged
abrisco merged 1 commit into
periareon:mainfrom
ifutivic:fix/import-canonical-reproducible-form
Jun 3, 2026
Merged

fix: return a valid canonical reproducible form from helm_import_repository#285
abrisco merged 1 commit into
periareon:mainfrom
ifutivic:fix/import-canonical-reproducible-form

Conversation

@ifutivic

@ifutivic ifutivic commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Issue

helm_import_repository's implementation returns the dict Bazel treats as the repository's "canonical reproducible form". Bazel compares each returned attribute against the declared one and emits a DEBUG message suggesting the user add any that differ.

url is mutually exclusive with repository / chart_name / version (the rule fail()s if both are set), but the implementation returned all of them unconditionally. So the suggested form is one the rule itself rejects:

  • importing via url → suggests adding chart_name / version
  • importing via repository → suggests adding url

Either way the user gets a confusing, unactionable DEBUG message. Fixes #189.

DEBUG: Rule '...' indicated that a canonical reproducible form can be obtained
by modifying arguments chart_name = "grafana", version = "12.1.4"

Change

Return only the attributes matching the source type actually used, so the suggested reproducible form is always a valid re-declaration:

  • url mode → {name, url, sha256}
  • repository mode → {name, repository, chart_name, version, sha256}

Testing

Verified with bazel fetch --repo=@... --force against the charts already in tests/test_deps.bzl:

Chart Source Before After
postgresql url= (http) DEBUG suggests chart_name/version clean
grafana url= (oci) DEBUG suggests chart_name/version clean
redis repository= DEBUG suggests url clean

Fixes #189

@ifutivic ifutivic force-pushed the fix/import-canonical-reproducible-form branch from c1ca5ad to 091a7b2 Compare June 3, 2026 07:30
The repository rule implementation returns the dict it considers the
"canonical reproducible form" of the repo. Bazel compares each returned
attribute against the declared one and emits a DEBUG message suggesting
the user add any that differ.

`url` is mutually exclusive with `repository`/`chart_name`/`version`, but
the implementation returned all of them unconditionally. As a result:

- importing via `url` suggested adding `chart_name`/`version`
- importing via `repository` suggested adding `url`

Both suggestions are rejected by the rule's own argument validation, so
the DEBUG message is confusing and unactionable (see periareon#189).

Return only the attributes that match the source type that was actually
used so the suggested reproducible form is always a valid re-declaration.

Fixes periareon#189
@ifutivic ifutivic force-pushed the fix/import-canonical-reproducible-form branch from 091a7b2 to ee0849b Compare June 3, 2026 07:31
@ifutivic

ifutivic commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

@abrisco Can you please take a look at the PR?

Should be a less disruptive fix for issue #189 compared to #196 which refactors the repo rules quite a lot.

@ifutivic

ifutivic commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Immediately after opening this PR I realized that rules_helm doesn't support the new repo_metadata return value. I stacked a second PR on top of this one which adds that as well: #286

@abrisco abrisco left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you!

@abrisco abrisco merged commit 24d14f4 into periareon:main Jun 3, 2026
13 checks passed
@ifutivic ifutivic deleted the fix/import-canonical-reproducible-form branch June 3, 2026 13:58
abrisco pushed a commit that referenced this pull request Jun 3, 2026
> [!NOTE]
> Stacked on #285. That PR fixes the *contents* of the reproducibility
dict, this one changes the *mechanism* used to return it. Review/merge
after #285 (or together).

## Issue

Bazel 9 deprecates returning a bare dict from a repository rule
implementation in favor of
[`repository_ctx.repo_metadata`](https://bazel.build/rules/lib/builtins/repository_ctx#repo_metadata).
Beyond replacing the dict, `repo_metadata` lets a repo declare itself
**reproducible**.

`helm_import_repository` already resolves a `sha256` for every fetch. A
chart pinned by `sha256` is reproducible by definition, so we mark it
`reproducible = True`. When no `sha256` was supplied we instead
advertise the resolved one via `attrs_for_reproducibility`, exactly as
the legacy bare-dict return did.

The call is guarded with `hasattr`, matching the approach in
bazel-contrib/rules_python#3597 and
bazel-contrib/rules_multitool#123:
`repo_metadata` was only added in Bazel 8.3.0, and this project still
supports Bazel 7.x in CI.

## Testing

Verified with `bazel fetch --repo=@... --force` on Bazel 9.1.0 against
the charts in `tests/test_deps.bzl`:

| Scenario | Result |
|---|---|
| `postgresql`/`grafana`/`redis` with `sha256` | `reproducible = True`,
no DEBUG, fetch cached |
| `postgresql` with `sha256` removed | DEBUG correctly suggests adding
`sha256 = "..."`, fetch succeeds |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Not consistent DEBUG messages using oci registry

2 participants