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

tests: enable testing with snapd snap FIPS variant #14476

Merged
merged 10 commits into from
Sep 16, 2024

Conversation

bboozzoo
Copy link
Contributor

@bboozzoo bboozzoo commented Sep 6, 2024

Based on #14439. Add spread testing of FIPS snap variant.

Related: SNAPDENG-23245

@bboozzoo bboozzoo force-pushed the bboozzoo/fips-snap-test branch 3 times, most recently from ce5476e to 9cc5ae2 Compare September 10, 2024 12:44
@bboozzoo bboozzoo marked this pull request as ready for review September 10, 2024 12:44
Copy link
Contributor

@zyga zyga left a comment

Choose a reason for hiding this comment

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

Bulk of the changes look good. I left a comment on a code comment that could use some clearer wording IMO.

LGTM when you want.

echo "-- appending FIPS tag to version $VERSION"
VERSION="$VERSION-fips"
if [ "${VERSION%/+fips/}" != "$VERSION" ] ; then
# we gave a '+fips' element in the version
Copy link
Contributor

Choose a reason for hiding this comment

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

Gave?

fi
# TODO detect when doing a FIPS snap build on LP
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a ticket for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@valentindavid valentindavid left a comment

Choose a reason for hiding this comment

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

A few comments

if [ "${VERSION%/+fips/}" != "$VERSION" ] ; then
# we have a '+fips' element in the version, which may be coming from
# debian/changelog or git tag
echo "-- deteceted FIPS build"
Copy link
Contributor

Choose a reason for hiding this comment

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

Detected?

if [ -f fips-build ] ; then
echo "-- appending FIPS tag to version $VERSION"
VERSION="$VERSION-fips"
if [ "${VERSION%/+fips/}" != "$VERSION" ] ; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean "${VERSION%+fips}" or "${VERSION/%+fips/}"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yes, too many changes going back and forth, /+fips/ let me fix that

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, so if it was supposed to be /+fips/ it does not have to end with then. Is that what you wanted?

Copy link
Contributor Author

@bboozzoo bboozzoo Sep 13, 2024

Choose a reason for hiding this comment

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

it's just a simplification, debian/rules checks simply checks for +fips too (not necessarily at the end), but we'll only ever append +fips as a suffix in teh changelog

# TODO detect when doing a FIPS snap build on LP

if [ -f fips-build ]; then
if [ "${VERSION%/+fips/}" = "$VERSION" ] ; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, did you mean "${VERSION%+fips}" or "${VERSION/%+fips/}"?

Extend the test to verify FIPS snapd support when it's running from the snapd snap.

Signed-off-by: Maciej Borzecki <[email protected]>
Download either a regular of a FIPS snap artifact depending on systems group the
test workflow has been started for.

Signed-off-by: Maciej Borzecki <[email protected]>
Do the same as we do for the deb.

Signed-off-by: Maciej Borzecki <[email protected]>
Signed-off-by: Maciej Borzecki <[email protected]>
Add a very ugly check for the presence of 1.21 FIPS toolchain.

Signed-off-by: Maciej Borzecki <[email protected]>
Set snapdfips build tag when building in FIPS mode.

Signed-off-by: Maciej Borzecki <[email protected]>
@bboozzoo
Copy link
Contributor Author

Failing tests:

  • google:ubuntu-20.04-64:tests/main/snap-remove-terminate - known, unrelated

@Meulengracht Meulengracht merged commit f3d0682 into canonical:master Sep 16, 2024
53 of 54 checks passed
@bboozzoo bboozzoo deleted the bboozzoo/fips-snap-test branch September 16, 2024 07:01
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.

4 participants