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

sign: Add support for ociarchive #2417

Merged
merged 1 commit into from
Sep 14, 2021
Merged

Conversation

cgwalters
Copy link
Member

Part of: coreos/fedora-coreos-tracker#812

We need to support signing ostree-native container images in
addition to our custom "ostree-archive-in-tar". To keep both
paths aligned, first export the archive (whether tar or ostree-container)
to an unpacked tmp/repo.

This repo then takes the place of the previous temporary repo where
we added a dummy remote to use to verify the signature generated.

Use public OSTree APIs to read/write commit metadata instead
of doing it by hand. But in the tar case, we keep the optimization of just
reflinking and appending to the archive.

@cgwalters
Copy link
Member Author

cgwalters commented Sep 7, 2021

I've only "compile" tested this. I may try adding a test path which mocks up the signature. But the strongest verification would look like this:

@cgwalters
Copy link
Member Author

Alternatively there is the --stg support; who/what has access to that?

jlebon
jlebon previously approved these changes Sep 8, 2021
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

LGTM overall though agreed we should test this before merging.

Alternatively there is the --stg support; who/what has access to that?

I have access to this (@dustymabe also does) but honestly it might still be easier to go the cosa rebuild route. You can just push it to a branch and add a Quay trigger. If you'd like to go the local way, I can send you the credentials for stage. The fedmsg.toml to use will look like this one: https://github.com/coreos/fedora-coreos-pipeline/blob/main/configs/fedmsg.toml, but the URL and TLS bits pointing at stage. And I think it'd be coreos.stg:, not coreos:... It's been a while since I've touched this stuff. Definitely an area that needs better documentation.

subprocess.check_call(['cp-reflink', exported_ostree_path, tmp_tar])
with tarfile.open(tmp_tar, 'a:') as t:
t.add(metapath, arcname=f'objects/{checksum[:2]}/{checksum[2:]}.commitmeta')
shutil.move(tmp_tar, exported_ostree_path)
Copy link
Member

Choose a reason for hiding this comment

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

Minor: would be nicer to have this be the last operation as before. (I.e. right before build.write()). Fine as is though since in practice a failure here just fails the whole build anyway.

@cgwalters
Copy link
Member Author

I pushed quay.io/cgwalters/coreos-assembler:sign-container with the code from this PR.

@jlebon
Copy link
Member

jlebon commented Sep 13, 2021

16:04:45  Sending org.fedoraproject.prod.coreos.build.request.ostree-sign with body {'build_id': '36.20210913.91.1', 'basearch': 'x86_64', 'commit_object': 's3://fcos-builds/prod/streams/rawhide/builds/tmp-x86_64/ostree-commit-object', 'checksum': 'sha256:0a760e70f18c2863bcccbd5361debce9580ec60e825573e2470e0f8cb7ad45f8', 'stream': 'rawhide', 'request_id': '78e3e6c1-7590-4101-80f4-8d92a190c099'}
16:04:47  Waiting for a response to the sent request
16:05:19  0 metadata, 0 content objects imported; 0 bytes content written
16:05:19  Verifying OSTree signature
16:05:19  Traceback (most recent call last):
16:05:19    File "/usr/lib/coreos-assembler/cmd-sign", line 273, in <module>
16:05:19      sys.exit(main())
16:05:19    File "/usr/lib/coreos-assembler/cmd-sign", line 42, in main
16:05:19      args.func(args)
16:05:19    File "/usr/lib/coreos-assembler/cmd-sign", line 95, in cmd_robosignatory
16:05:19      robosign_ostree(args, s3, build, gpgkey)
16:05:19    File "/usr/lib/coreos-assembler/cmd-sign", line 149, in robosign_ostree
16:05:19      repo.write_commit_detached_metadata(checksum, commitmeta_data, None)
16:05:19  gi.repository.GLib.Error: g-io-error-quark: Bad file descriptor (0)

@cgwalters
Copy link
Member Author

Should be fixed - will watch for the quay rebuild

@cgwalters
Copy link
Member Author

quay.io/cgwalters/coreos-assembler@sha256:0eaadb6d9a4e8c075ff46bb6f64754ff01fce5224709c8d7ef6abe2effac360a

@jlebon
Copy link
Member

jlebon commented Sep 14, 2021

@jlebon
Copy link
Member

jlebon commented Sep 14, 2021

21:04:14  Verifying OSTree signature
21:04:14  Valid signature from Fedora <fedora-36-primary@fedoraproject.org> (53DED2CB922D8B8D9E63FD18999F7CBF38AB71F4)
21:04:14  Traceback (most recent call last):
21:04:14    File "/usr/lib/coreos-assembler/cmd-sign", line 274, in <module>
21:04:14      sys.exit(main())
21:04:14    File "/usr/lib/coreos-assembler/cmd-sign", line 42, in main
21:04:14      args.func(args)
21:04:14    File "/usr/lib/coreos-assembler/cmd-sign", line 95, in cmd_robosignatory
21:04:14      robosign_ostree(args, s3, build, gpgkey)
21:04:14    File "/usr/lib/coreos-assembler/cmd-sign", line 189, in robosign_ostree
21:04:14      with tarfile.open(tmp_tar, 'a:') as t:
21:04:14    File "/usr/lib64/python3.9/tarfile.py", line 1629, in open
21:04:14      return func(name, filemode, fileobj, **kwargs)
21:04:14    File "/usr/lib64/python3.9/tarfile.py", line 1659, in taropen
21:04:14      return cls(name, mode, fileobj, **kwargs)
21:04:14    File "/usr/lib64/python3.9/tarfile.py", line 1474, in __init__
21:04:14      fileobj = bltn_open(name, self._mode)
21:04:14  PermissionError: [Errno 13] Permission denied: 'tmp/cosa-signrw2wv9if/fedora-coreos-36.20210914.91.0-ostree.x86_64.tar'

Part of: coreos/fedora-coreos-tracker#812

We need to support signing ostree-native container images in
addition to our custom "ostree-archive-in-tar".  To keep both
paths aligned, first export the archive (whether tar or ostree-container)
to an unpacked `tmp/repo`.

This repo then takes the place of the previous temporary repo where
we added a dummy remote to use to verify the signature generated.

Use public OSTree APIs to read/write commit metadata instead
of doing it by hand.  But in the tar case, we keep the optimization of just
reflinking and appending to the archive.
@cgwalters
Copy link
Member Author

OK whee, another one liner.

Glad we're doing this!

@cgwalters
Copy link
Member Author

quay.io/cgwalters/coreos-assembler@sha256:4849f86668014272a5ceb258d0371bd6c8aef4a76fa74d03e1c5551ea283001e

@jlebon
Copy link
Member

jlebon commented Sep 14, 2021

jenkins-fedora-coreos.apps.ocp.ci.centos.org/job/fedora-coreos/job/fedora-coreos-fedora-coreos-pipeline/2690

This one is still going, but it did go past OSTree signing successfully already, so I think we're good to go!

CI looks like a flake. We should dig into it, but restarted it for now.

@cgwalters cgwalters enabled auto-merge (rebase) September 14, 2021 18:36
@jlebon
Copy link
Member

jlebon commented Sep 14, 2021

Filed coreos/fedora-coreos-tracker#961 for the flake.

@cgwalters cgwalters merged commit 50bd41d into coreos:main Sep 14, 2021
cgwalters added a commit to cgwalters/fedora-coreos-config that referenced this pull request Sep 14, 2021
(Take 2, now that we have coreos/coreos-assembler#2417 )

Part of coreos/fedora-coreos-tracker#812

In this initial step, we're merely switching the internal
tarball to be a different format.

A future step will change the FCOS pipeline to automatically
push this container to quay.io.
jlebon pushed a commit to coreos/fedora-coreos-config that referenced this pull request Sep 15, 2021
(Take 2, now that we have coreos/coreos-assembler#2417 )

Part of coreos/fedora-coreos-tracker#812

In this initial step, we're merely switching the internal
tarball to be a different format.

A future step will change the FCOS pipeline to automatically
push this container to quay.io.
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