Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor composefs warnings #2994

Merged
merged 4 commits into from
Aug 28, 2023

Conversation

cgwalters
Copy link
Member

prepare-root: Drop redundant print about signature/digest

We print if we're doing a signature+digest verification; its absence is
sufficient in the other case. The goal here is to avoid polluting
the logs when signatures are not enabled.


prepare-root: Init composefs options earlier

Prep for a later patch.


prepare-root: Fold together composefs signature cases

Now that we don't support digest-but-not-signature verification
for composefs, the logic here was unnecessarily complicated.
With a prior prep patch that moved the composefs option
initialization up, we can just have everything related to signature
verification in a single conditonal.


composefs: Hard error except on ENOENT even in "optional" case

Since we enabled composefs at build time, the default (non-composefs)
case now always prints
composefs: Optional support failed: No such file or directory
But that's normal and expected.

Rework things here so that in the very special case where
we are in "maybe/optional" mode and we get ENOENT, then we
output a much more normal-looking message that doesn't include
the string "failed".

Now on the flip side - if I have explicitly enabled signature
checking, I think we do want to make that fatal even if
composefs is in "maybe" mode.

(This part is more debatable; perhaps we should just disallow
the case of "maybe" + signatures at all; but I think this is
an improvement in that direction)


@cgwalters
Copy link
Member Author

@alexlarsson review ping if you have a bit of capacity, I think this one is quite safe and it passes the tests we have here (though we don't yet cover the signed path in CI)

@cgwalters
Copy link
Member Author

I guess we can hold this one for the next release, since it could use a bit more testing and in the end it's mostly cosmetic.

@cgwalters cgwalters marked this pull request as draft August 24, 2023 12:31
@cgwalters cgwalters marked this pull request as ready for review August 25, 2023 15:35
@cgwalters
Copy link
Member Author

OK release is done, lifting draft here

We print if we're doing a signature+digest verification; its absence is
sufficient in the other case.  The goal here is to avoid polluting
the logs when signatures are not enabled.
Now that we don't support digest-but-not-signature verification
for composefs, the logic here was unnecessarily complicated.
With a prior prep patch that moved the composefs option
initialization up, we can just have everything related to signature
verification in a single conditonal.
Since we enabled composefs at build time, the default (non-composefs)
case now always prints
`composefs: Optional support failed: No such file or directory`
But that's normal and expected.

Rework things here so that in the very special case where
we are in "maybe/optional" mode and we get ENOENT, then we
output a much more normal-looking message that doesn't include
the string "failed".

Now on the flip side - if I have explicitly enabled signature
checking, I think we *do* want to make that fatal even if
composefs is in "maybe" mode.

(This part is more debatable; perhaps we should just disallow
 the case of "maybe" + signatures at all; but I think this is
 an improvement in that direction)
@cgwalters cgwalters merged commit 89e13a9 into ostreedev:main Aug 28, 2023
24 checks passed
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.

None yet

2 participants