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

(maint) Fix duplicate solaris ca cert #207

Merged
merged 2 commits into from
Nov 1, 2022
Merged

Conversation

yachub
Copy link

@yachub yachub commented Nov 1, 2022

What's this PR do?

The DigiCert root CA G4 cert is named DigiCert_Trusted_Root_G4.pem on a fully patched Solaris 11.4 system. This resolves a duplicate certificate error after restarting ca-certificates.

Should any of this be tested outside the normal PR CI cycle?

Tested on jenkins staging instance.

Any background context you want to provide?

Solaris 11 test targets recently updated to fully patched 11.4 install

Questions for reviewers?

Prefer keeping this method to support older Solaris installs, or remove completely?

The DigiCert root CA G4 cert is named `DigiCert_Trusted_Root_G4.pem` on a fully patched Solaris 11.4 system. This resolves a duplicate certificate error after restarting ca-certificates.
@yachub yachub force-pushed the maint/fix_duplicate_solaris_cert branch from f1e418f to 0fe5624 Compare November 1, 2022 12:27
@yachub yachub marked this pull request as ready for review November 1, 2022 14:00
@yachub yachub requested a review from a team as a code owner November 1, 2022 14:00
CODEOWNERS Outdated Show resolved Hide resolved
@yachub
Copy link
Author

yachub commented Nov 1, 2022

Same change made at puppetlabs/beaker-puppet#202 as well.

@nmburgan
Copy link
Contributor

nmburgan commented Nov 1, 2022

LGTM assuming it all tests out okay. Could you add @puppetlabs/skeletor and @puppetlabs/nimbus as owners too?

@yachub yachub added the bug label Nov 1, 2022
@@ -1396,10 +1396,10 @@ def solaris_key_chain_fix
EOM
hosts.each do |host|
if host.platform=~ /solaris-11(\.2)?-(i386|sparc)/
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need x86_64 in here, or is solaris11-64f somehow i386?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can probably remove the (\.2)?

Copy link
Author

Choose a reason for hiding this comment

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

Totally ignorant here, Is i386 A reference vanagon? Like https://github.com/puppetlabs/vanagon/blob/80851591ef40c14b439d4a63a91f68d2d0526790/lib/vanagon/platform/defaults/solaris-11-i386.rb actually points to x86_64?

Ya I was on the fence about removing the (.2) since we hadn't actually destroyed the old pools yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't remember why, but we use solaris-11-i386 as the name of the vanagon platform to build on: https://github.com/puppetlabs/puppet-agent/blob/main/configs/platforms/solaris-11-i386.rb

And that is reflected in the beaker-hostgenerator data:

$ bx beaker-hostgenerator solaris11-64a 
---
HOSTS:
  solaris11-64-1:
    platform: solaris-11-i386
    template: solaris-11-x86_64
    hypervisor: vmpooler
    roles:
    - agent

I assume this step is trying to match 11 or 11.2 but not 11.4. Presumably the cert doesn't need updating on that platform?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is an example of why I'm not a big fan of making the solaris-11-x86_64 pool actually be 11.4. There are probably lots of crusty assumptions built up that solaris-11-x86_64 is < 11.4

Copy link
Author

@yachub yachub Nov 1, 2022

Choose a reason for hiding this comment

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

I guess I figured it was a good time to rip the bandaid off since 11.4 is the final release and 11.1 and 11.2 aren't supported anymore. Plus then it will match the solaris-11-sparc pool not having specific point releases (and matches the majority of other pools).

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, it looks like solaris11-64 has solaris-11-i386 underneath, but the vmpooler platform is solaris-11-x86_64. Weird.

https://github.com/voxpupuli/beaker-hostgenerator/blob/69a61044edb40b763f8bcc3ca8048c84c41bee01/lib/beaker-hostgenerator/data.rb#L1198-L1205

Since we're interested in platform, looks like i386 is still fine.

Copy link
Author

Choose a reason for hiding this comment

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

I would really like to get this if possible though so that nightly tests get fixed.

Copy link
Author

@yachub yachub Nov 1, 2022

Choose a reason for hiding this comment

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

Either way this change is backwards compatible, so it will still work if we have to revert the pools for some reason, but I'll release it as 2.11.24

@nmburgan
Copy link
Contributor

nmburgan commented Nov 1, 2022

Added a comment

@yachub
Copy link
Author

yachub commented Nov 1, 2022

LGTM assuming it all tests out okay. Could you add @puppetlabs/skeletor and @puppetlabs/nimbus as owners too?

@nmburgan Sure, do you think there's any reason to break out specific directories? Like just "*" for all 3 teams? Or add those two teams to the existing specific paths?

@nmburgan
Copy link
Contributor

nmburgan commented Nov 1, 2022

Probably not, I think you can give it all to us and RE

@yachub yachub force-pushed the maint/fix_duplicate_solaris_cert branch from 0e67b5d to 51429d9 Compare November 1, 2022 21:17
@yachub yachub merged commit f3222d9 into main Nov 1, 2022
@yachub yachub deleted the maint/fix_duplicate_solaris_cert branch May 10, 2023 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants