Skip to content

test(aws): cover the marketplace listing gate, and correct the rule it broke - #614

Merged
cevheri merged 2 commits into
mainfrom
fix/aws-gate-tests
Sep 7, 2026
Merged

cevheri merged 2 commits into
mainfrom
fix/aws-gate-tests

Conversation

@yusuf-gundogdu

Copy link
Copy Markdown
Member

Two files. The channel gate you added in 28d74ec now has tests, and the paragraph under the channels table no longer contradicts the row you added in the same commit.

The tests

The gate shipped without any, so nothing held the mutations that matter. Each of these now fails the suite, and I checked every one by making the edit and re-running rather than by reading:

  • Dropping the awk's - id: bound. A row missing its status: then makes the gate read the next channel's - winget's, live - and open.
  • Flipping the named-version exemption. That is the path the first AMI is built on, the one the submission itself needs, so inverting it locks the product out of ever being listed.
  • Lifting stand_down out of the exemption block while keeping every substring in place. Same lockout, spelled differently.
  • Deleting the exit 0 from stand_down(), which would turn every stand-down in the step into a no-op because the later run=true wins.
  • Inverting the explicit= arms, or letting INPUT_VERSION fall back to the release tag: either makes a plain release look like a person and builds while the channel is not live.
  • Moving the gate below run=true.

The channels.yaml row is asserted the way the awk matches it: anchored, and bounded to its own block. A substring test would pass for aws-marketplace-anything, which the workflow would never find, and an unbounded slice would read the next channel's status and pass whatever this one said.

The paragraph

Below the table it said a row appears "the moment a submission exists", and then that AWS Marketplace has "no submitted product, so there is nothing to track yet" - three sentences apart, with the row between them. The rule is the half that was wrong: a row appears as soon as there is something to track, which includes a descriptor in this repo that a workflow reads, and stays pending until the product can be installed. That is what the Azure row has always been, and now the AWS one too.

Worth knowing

The gate exempts a run that names a version, and dispatch-downstream passes -f "version=$TAG" to the workflows it chains. If aws-ami-build joins that chain in the same idiom, explicit is yes and the gate only prints a notice. Chaining it without a version keeps the gate live - deploy/aws/README.md already describes it that way.

Checks: 8629 unit tests pass, format, lint, typecheck, knip, distribution matrix, showcase, readme and security guards all clean.

…t broke

The gate landed in 28d74ec without tests, so nothing held the two
mutations that matter. Both now fail the suite:

- dropping the awk's `- id:` bound. A row missing its `status:` then
  makes the gate read the NEXT channel's - winget's, `live` - and open.
- flipping the named-version exemption. That is the path the first AMI is
  built on, the one the submission itself needs, so inverting it locks
  the product out of ever being listed.

The row the gate reads is asserted the way the awk matches it, anchored
and bounded to its own block: a substring test would pass for
`aws-marketplace-anything`, which the workflow would never find, and an
unbounded slice would read the next channel's status and pass whatever
this one said. Its category is pinned too, because a row moved out of
cloud-marketplaces stops being counted while the gate keeps reading it.

docs/CHANNELS.md, below the table, said a row appears "the moment a
submission exists" and then, in the same paragraph, that AWS Marketplace
had no submitted product and nothing to track. Both cannot be true now
that the row exists and a workflow reads it. The rule is the half that
was wrong: a row appears as soon as there is something to track, which
includes a descriptor in this repo, and stays `pending` until the product
can be installed.
The second real build got as far as `02-configure.sh` and refused a
correct image: "effective sshd config still permits root password login".

The check was an allow-list - `no|prohibit-password|forced-commands-only`
- and the value sshd reports is not the value you wrote: every OpenSSH
since 7.0 prints `without-password`, the deprecated synonym, because that
spelling comes first in its multistate table. The image was right; the
assertion was wrong about how the answer is spelled.

`yes` is the only value that permits a root password login, and it has no
synonym to miss, so the check now looks for that and fails on it. It also
prints the line it saw - the allow-list version could not, which is why
diagnosing its misfire cost a whole second build - and `sshd -T` is now
captured once into a variable, which fails closed on its own under
`set -e` and is read twice without running sshd twice.

The test pins the whole construct rather than the grep's text, so an
inverted or gutted check fails, and it rejects an allow-list as a class
rather than the one spelling that was there before.
@yusuf-gundogdu

Copy link
Copy Markdown
Member Author

Second commit added: the AMI build's sshd assertion was rejecting a correct image.

The check was an allow-list - no|prohibit-password|forced-commands-only - and the value sshd reports is not the value you write: every OpenSSH since 7.0 prints without-password, the deprecated synonym, because that spelling comes first in its multistate table. Run 34066747704 got through the image pull and the file upload and then died in 02-configure.sh with "effective sshd config still permits root password login" on an image whose drop-in was correct.

yes is the only value that permits a root password login and it has no synonym, so the check now fails on that instead. It prints the line it saw, and sshd -T is captured once into a variable, which fails closed on its own under set -e.

Three agents reviewed both commits. What they caught, all fixed before this push: the test asserted the grep's text rather than the whole construct, so an inverted or gutted check passed; the allow-list guard pinned one spelling rather than the pattern; and a comment attributed the synonym to the base image rather than to OpenSSH.

@sonarqubecloud

sonarqubecloud Bot commented Sep 7, 2026

Copy link
Copy Markdown

@cevheri
cevheri merged commit 6447d25 into main Sep 7, 2026
30 checks passed
@cevheri
cevheri deleted the fix/aws-gate-tests branch September 7, 2026 06:29
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