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

Implement Package Signing #214

Merged
merged 28 commits into from
May 1, 2024

Conversation

pmengelbert
Copy link
Contributor

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Fixes #

Special notes for your reviewer:

cmd/signer/Dockerfile Outdated Show resolved Hide resolved
@pmengelbert pmengelbert force-pushed the pmengelbert/package_signing/5 branch from 1f8d93f to 4c4edd1 Compare April 19, 2024 18:51
cmd/signer/main.go Outdated Show resolved Hide resolved
test/fixtures/phony.go Outdated Show resolved Hide resolved
test/testenv/buildx.go Outdated Show resolved Hide resolved
test/testenv/builld.go Outdated Show resolved Hide resolved
frontend/mariner2/handle_rpm.go Outdated Show resolved Hide resolved
frontend/request.go Outdated Show resolved Hide resolved
frontend/request.go Show resolved Hide resolved
frontend/request.go Outdated Show resolved Hide resolved
@pmengelbert pmengelbert force-pushed the pmengelbert/package_signing/5 branch from 26a3286 to bca560c Compare April 29, 2024 17:03
docs/signing.md Outdated Show resolved Hide resolved
The general idea is that the image specified in
spec.targets.x.package_config.signer.image will receive an `llb.State`
as an input, and provide a tranformed state (with the artifacts in
question signed in the desired manner) as the output.

Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
I had been using debugging symbols to fix some issues in the
integration tests.

Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Instead of telling the frontend how to search for artifacts to sign,
only provide `dalec.target` and delegate responsibility for finding
the artifacts to the signing side. The target should be sufficient to
determine what to do.

Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
@pmengelbert pmengelbert force-pushed the pmengelbert/package_signing/5 branch from 72c6130 to accc429 Compare April 30, 2024 17:55
Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

This is looking really close!

signOp = linuxSignOp
}

config := Config{
Copy link
Member

Choose a reason for hiding this comment

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

Can we simplify this down to something minimal for the test fixture?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is done.

test/signer_test.go Outdated Show resolved Hide resolved
* Move signing tests to mariner2_test and windows_test, respectively,
since the test is checking 1) that the forwarding happens and 2) that
the signing frontend receives sufficient information to determine
signing logic.
* Simplify the test signing frontend to do less.

Signed-off-by: Peter Engelbert <[email protected]>
This allows the same signer to be configured for all distros in the spec.

Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
},
}

sr := newSolveRequest(withSpec(ctx, t, &spec), withBuildTarget("mariner2/rpm"))
Copy link
Member

Choose a reason for hiding this comment

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

This is specific to mariner, but we should treat this as a more generic test so that we can add other distros (azlinux3, jammy, etc) and get these tests for free.
Maybe we need to add an extra parameter to the test function to specify the signing target.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made this generic. LMK what you think

Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
@cpuguy83 cpuguy83 added this to the v0.3.0 milestone May 1, 2024
@cpuguy83 cpuguy83 merged commit 92bcfa2 into Azure:main May 1, 2024
10 checks passed
@cpuguy83 cpuguy83 mentioned this pull request May 23, 2024
1 task
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