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

fix(24.04): reorganise ca-certificates slice(s) #266

Conversation

zhijie-yang
Copy link
Collaborator

Proposed changes

The script update-ca-certificates and its corresponding config file /etc/ca-certificates.conf is added to the SDF of ca-certificates.

Related issues/PRs

ROCKS-994

Forward porting

Not applicable.

Checklist

Copy link

github-actions bot commented Jun 24, 2024

Diff of dependencies:

slices/ca-certificates.yaml
@@ -1,3 +1,2 @@
-debconf
-debconf-2.0
 openssl
+sed

@zhijie-yang zhijie-yang force-pushed the ROCKS-994-ca-cert-mutation-scripts-24.04 branch from aa0c16c to f45dba9 Compare June 24, 2024 10:22
@zhijie-yang zhijie-yang force-pushed the ROCKS-994-ca-cert-mutation-scripts-24.04 branch from f45dba9 to f2eee66 Compare June 27, 2024 10:13
@zhijie-yang zhijie-yang force-pushed the ROCKS-994-ca-cert-mutation-scripts-24.04 branch from f2eee66 to 0417cb0 Compare July 3, 2024 07:30
@zhijie-yang zhijie-yang force-pushed the ROCKS-994-ca-cert-mutation-scripts-24.04 branch from 0417cb0 to 77025ed Compare July 4, 2024 10:49
slices/ca-certificates.yaml Outdated Show resolved Hide resolved
slices/ca-certificates.yaml Show resolved Hide resolved
slices/ca-certificates.yaml Show resolved Hide resolved
slices/ca-certificates.yaml Show resolved Hide resolved
slices/ca-certificates.yaml Outdated Show resolved Hide resolved
slices/ca-certificates.yaml Show resolved Hide resolved
slices/ca-certificates.yaml Show resolved Hide resolved
@rebornplusplus rebornplusplus added the Priority Look at me first label Jul 5, 2024
@zhijie-yang zhijie-yang force-pushed the ROCKS-994-ca-cert-mutation-scripts-24.04 branch from a853570 to 0a06b57 Compare July 5, 2024 15:22
slices/ca-certificates.yaml Outdated Show resolved Hide resolved
@zhijie-yang zhijie-yang requested a review from cjdcordeiro July 8, 2024 12:44
@zhijie-yang zhijie-yang requested a review from linostar July 8, 2024 12:44
Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

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

Nice. LGTM. if @rebornplusplus and @javierdelapuente approve, I'll merge it

@javierdelapuente
Copy link

javierdelapuente commented Jul 8, 2024

Not sure if related to this PR, or the way I am doing it is different from the way it is really done while building a rock. I am trying to use ca-certificates_data-with-certs from this PR using this rockcraft.yaml file: https://pastebin.ubuntu.com/p/gDYN6njnc9/

I get the following error (I think the other cert.pem file comes as a dependency of python3-venv):

2024-07-08 15:43:07.175 :: 2024-07-08 15:43:06.633 Failed to stage: parts list the same file with different contents or permissions.
2024-07-08 15:43:07.175 :: 2024-07-08 15:43:06.633 Detailed information: Parts 'django-framework/runtime' and 'django-framework/dependencies' list the following files, but with different contents or permissions:
...
2024-07-08 15:43:07.175 :: 2024-07-08 15:43:06.639 Parts 'django-framework/runtime' and 'django-framework/dependencies' list the following files, but with different contents or permissions:
2024-07-08 15:43:07.175 :: 2024-07-08 15:43:06.639     usr/lib/ssl/cert.pem
...

Checking inside lxc I see:

rockcraft-netbox-on-amd64-for-amd64-15 ../parts/django-framework# find . -name "cert.pem" -ls
  2147356      0 lrwxrwxrwx   1 root     root           34 Jul  8 15:42 ./runtime/install/usr/lib/ssl/cert.pem -> /etc/ssl/certs/ca-certificates.crt
  2103683      0 lrwxrwxrwx   1 root     root           42 Jul  8 15:35 ./dependencies/install/usr/lib/ssl/cert.pem -> ../../../etc/ssl/certs/ca-certificates.crt

Will this also happen when rockcraft gets the chiselled ca-certificates_data-with-certs?

Copy link
Member

@rebornplusplus rebornplusplus left a comment

Choose a reason for hiding this comment

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

Sorry to go back and forth on this. It looks quite nice. Just have one concern about the ca-certificates.conf I mentioned below.

In general, I see two options about the whole ca-certificates offering:

  1. The data slice provides the bundle, individual default certs and the config file. Pro: this is a standard offering and doesn't break anything. Con: the slice doubles in size and then some, for the config file. Another slice (name TBD) provides only the bundle and nothing else.
  2. As it is now, the data slice provides only the bundle and another slice provides the bundle, config and the individual default certs.

Re-considering it, I feel much more inclined to having option 1 since most of the users would not mind the small extra weight in data slice as long it doesn't break and provide a smooth experience. Let me know what you think!

slices/ca-certificates.yaml Outdated Show resolved Hide resolved
slices/ca-certificates.yaml Outdated Show resolved Hide resolved
@rebornplusplus
Copy link
Member

Not sure if related to this PR, or the way I am doing it is different from the way it is really done while building a rock. I am trying to use ca-certificates_data-with-certs from this PR using this rockcraft.yaml file: https://pastebin.ubuntu.com/p/gDYN6njnc9/

I get the following error (I think the other cert.pem file comes as a dependency of python3-venv):

2024-07-08 15:43:07.175 :: 2024-07-08 15:43:06.633 Failed to stage: parts list the same file with different contents or permissions.
2024-07-08 15:43:07.175 :: 2024-07-08 15:43:06.633 Detailed information: Parts 'django-framework/runtime' and 'django-framework/dependencies' list the following files, but with different contents or permissions:
...
2024-07-08 15:43:07.175 :: 2024-07-08 15:43:06.639 Parts 'django-framework/runtime' and 'django-framework/dependencies' list the following files, but with different contents or permissions:
2024-07-08 15:43:07.175 :: 2024-07-08 15:43:06.639     usr/lib/ssl/cert.pem
...

Checking inside lxc I see:

rockcraft-netbox-on-amd64-for-amd64-15 ../parts/django-framework# find . -name "cert.pem" -ls
  2147356      0 lrwxrwxrwx   1 root     root           34 Jul  8 15:42 ./runtime/install/usr/lib/ssl/cert.pem -> /etc/ssl/certs/ca-certificates.crt
  2103683      0 lrwxrwxrwx   1 root     root           42 Jul  8 15:35 ./dependencies/install/usr/lib/ssl/cert.pem -> ../../../etc/ssl/certs/ca-certificates.crt

Will this also happen when rockcraft gets the chiselled ca-certificates_data-with-certs?

Hi @javierdelapuente, I think the other cert.pem file is coming from openssl being a dependency of python3-venv. If that's the case, the error above should not happen since the package path has an absolute path as the symlink target. Which makes me think there are two possible scenarios -- a) the file is not coming from openssl or b) somehow the symlink target is changed to a relative path, either by some other package or rockcraft (? doubt that).

In any case, no I don't think merging the slices would fix this issue. I reckon there's a need to dive deep and see where the symlink is coming from and/or where it's being modified.

Copy link

@linostar linostar left a comment

Choose a reason for hiding this comment

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

Reviewed and tested it with James. LGTM.

Copy link
Member

@rebornplusplus rebornplusplus left a comment

Choose a reason for hiding this comment

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

Looks nice to me. Only left a few suggestions about the comments.

slices/ca-certificates.yaml Outdated Show resolved Hide resolved
slices/ca-certificates.yaml Outdated Show resolved Hide resolved
slices/ca-certificates.yaml Outdated Show resolved Hide resolved
Co-authored-by: Rafid Bin Mostofa <[email protected]>
Copy link
Member

@rebornplusplus rebornplusplus left a comment

Choose a reason for hiding this comment

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

All good from me! Can be merged after another maintainer's approval.

@rebornplusplus rebornplusplus changed the title Rocks 994 ca cert mutation scripts 24.04 fix(24.04): reorganise ca-certificates slice(s) Jul 9, 2024
@rebornplusplus rebornplusplus self-assigned this Jul 9, 2024
@cjdcordeiro cjdcordeiro merged commit 06b5631 into canonical:ubuntu-24.04 Jul 11, 2024
12 checks passed
cjdcordeiro added a commit to cjdcordeiro/chisel-releases that referenced this pull request Jul 17, 2024

---------

Co-authored-by: Rafid Bin Mostofa <[email protected]>
Co-authored-by: Cristovao Cordeiro <[email protected]>
cjdcordeiro added a commit to cjdcordeiro/chisel-releases that referenced this pull request Jul 17, 2024

---------

Co-authored-by: Rafid Bin Mostofa <[email protected]>
Co-authored-by: Cristovao Cordeiro <[email protected]>
cjdcordeiro added a commit to cjdcordeiro/chisel-releases that referenced this pull request Jul 17, 2024

---------

Co-authored-by: Rafid Bin Mostofa <[email protected]>
Co-authored-by: Cristovao Cordeiro <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority Look at me first
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants