Skip to content

Fix invalid --plain-http flag on oras cp - #774

Open
bschwedler wants to merge 1 commit into
mainfrom
fix/oras-cp-plain-http
Open

Fix invalid --plain-http flag on oras cp#774
bschwedler wants to merge 1 commit into
mainfrom
fix/oras-cp-plain-http

Conversation

@bschwedler

Copy link
Copy Markdown
Contributor

oras cp has two endpoints, so unlike every other oras subcommand it takes --from-plain-http and --to-plain-http rather than a single --plain-http. OrasCopy.command emitted the bare form whenever plain_http was set:

$ oras cp --plain-http localhost:5601/src:v1 localhost:5602/dest:v1
Error: unknown flag: --plain-http

Confirmed against oras 1.3.3, whose cp --help lists only the per-endpoint flags. Sibling subcommands (manifest fetch, manifest index create) do accept --plain-http, so OrasCommand's shared field is correct everywhere except OrasCopy.

Each flag is now emitted only when that end is actually a registry — an OCI layout is a filesystem path, and a plain-HTTP flag aimed at it is meaningless:

from_oci_layout to_oci_layout Flags
false false --from-plain-http --to-plain-http
true false --to-plain-http only
false true --from-plain-http only

Impact

Production is unaffected — plain_http is only set for local registries. But any local-registry publish path failed immediately, which covers local development, the oven, and integration tests publishing against a registry:3 container.

Why the tests did not catch it

test_copy_with_plain_http covered this flag and passed, because it only asserted command construction — oras is never invoked, so it asserted the broken form was built correctly.

That gap is not fully closed here. Unit tests cannot validate flag validity; only running oras can. This PR updates the assertion, adds a regression guard that no layout combination emits the bare flag, and covers dropping the flag aimed at a layout endpoint. An integration test that actually invokes oras against test/helpers/registry_container.py would be the real fix, and is worth considering separately.

Verification

  • ruff check and ruff format --check clean
  • pytest test/plugins/builtin/imagetools/ → 134 passed
  • The single failure, TestFindOrasBin::test_find_from_path, is pre-existing and environmental (oras not on PATH); confirmed by running the same suite on unmodified code (60 passed → 62 passed with this change, same one failure)

Found while validating the retained-OCI-layout publish change.

oras cp has two endpoints, so unlike every other oras subcommand it takes
--from-plain-http and --to-plain-http rather than a single --plain-http.
OrasCopy emitted the bare form whenever plain_http was set, which exits
with "unknown flag" against oras 1.3.3.

Each flag is now emitted only when that end is actually a registry, since
an OCI layout is a filesystem path and a plain-HTTP flag aimed at it is
meaningless.

Production was unaffected because plain_http is only set for local
registries, but any local-registry publish path failed immediately,
covering local development, the oven, and integration tests that publish
against a registry container.

The existing test asserted the broken form and passed, because it only
checked command construction and never invoked oras. It now asserts the
per-endpoint flags, alongside a regression guard that no layout
combination emits the bare flag and coverage for dropping the flag aimed
at a layout endpoint.
@github-actions

Copy link
Copy Markdown

Test Results

2 258 tests   2 258 ✅  11m 37s ⏱️
    1 suites      0 💤
    1 files        0 ❌

Results for commit a6c5429.

@ianpittwood ianpittwood 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.

LGTM, but this may need to be surfaced to bakery ci publish too? Tough to say since this is more granular per-repo. We may have to consider altering the model for registries to specify http or something.

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.

2 participants