Skip to content

[match] select cert with the most furthest expiration date in the future#21809

Draft
nekrich wants to merge 16 commits into
fastlane:masterfrom
nekrich:match/improve-cert-selection
Draft

[match] select cert with the most furthest expiration date in the future#21809
nekrich wants to merge 16 commits into
fastlane:masterfrom
nekrich:match/improve-cert-selection

Conversation

@nekrich

@nekrich nekrich commented Jan 16, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I see several green ci/circleci builds in the "All checks have passed" section of my PR (connect CircleCI to GitHub if not)
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.
  • I've added or updated relevant unit tests.

Motivation and Context

In most cases, there is only one cert/key pair in the match storage.
But when dealing with developer_id certificates, there could be two or even three. The reason for this is that developer ID certs have a 5-year lifespan. And it looks like an app signed with developer_id cert stops working when the developer id cert used to sign it is expired. Every couple of years, we issue a new developer id cert and use it to sign new app versions. In this case, we are sure that the latest release will work for a couple more years, and users will have plenty of time to update the app.

Since the developer id cert is not expired we use manual import. After that, we have two cert/key pairs in the match storage.

The problem I want to address is that the match uses a cert with the biggest id (filename). And this is wrong. We need a match to select a cert with the furthest expiration date in the future.

Description

  1. select_cert_or_key is now select_cert. The method returns the cert with the furthest expiration date.
  2. New runner_unit_spec not to mix unit tests with looks like integration tests in the runner_spec.
  3. There could be several cert/key pairs in the storage. Refactored cert deletion logic to address this case. There is no value in expired certs at all.

Cert renewal updates

  1. cert_paths contain only valid certificates. It looks better and requires fewer checks in the code afterward.
  2. There is no need for the renew_expired_certs option 😅.

Other improvements

  1. There is no need in keys variable. We need only certs with corresponding p12 keys.
  2. Updates some gsubs to use regex, indicating the correct file extension and the end of a string.

PS. Looks like failing tests on Ubuntu is not related to my changes.

Testing Steps

Run the match with storage having 2 or more certs of the same type.
Run the match with storage having 2 or more certs that are expired.

@nekrich nekrich changed the title [match] select cert with the most furthest expiration date in the future [wip] [match] select cert with the most furthest expiration date in the future Jan 16, 2024
@nekrich nekrich changed the title [wip] [match] select cert with the most furthest expiration date in the future [match] select cert with the most furthest expiration date in the future Jan 16, 2024
@nekrich nekrich marked this pull request as ready for review January 16, 2024 12:25
@nekrich

nekrich commented Jan 16, 2024

Copy link
Copy Markdown
Contributor Author

@rogerluan, I would appreciate feedback on this. At some point, it slightly changes the renewal certs behavior introduced in #21691.

@lacostej

lacostej commented Jan 16, 2024

Copy link
Copy Markdown
Collaborator

Well if that removes the just freshly added renew_expired_certs option, we need to either remove the previous PR before making a new release, or delay the release until we land this one, right?

@nekrich

nekrich commented Jan 16, 2024

Copy link
Copy Markdown
Contributor Author

I'm good with both options )

@nekrich

nekrich commented Jan 16, 2024

Copy link
Copy Markdown
Contributor Author

I can create a small PR that removes the option, but the logic stays the same. Give me a couple of mins

@nekrich nekrich mentioned this pull request Jan 16, 2024
Merged
6 tasks
@nekrich

nekrich commented Jan 16, 2024

Copy link
Copy Markdown
Contributor Author

And new PR: #21812

@nekrich nekrich marked this pull request as draft February 8, 2024 00:16
@nekrich

nekrich commented Feb 8, 2024

Copy link
Copy Markdown
Contributor Author

Hi, @rogerluan and @lacostej. We are experiencing some issues with encryption. I don't know at this moment if it's this PR, or #21691. It's a double encryption or no encryption at all, idk. I think it's better to revert to #21691, and I'll be able to return back to this PR in 2 weeks. I'll try to make it faster, but no promises. Sorry for the bother 😥.

@lacostej

lacostej commented Feb 9, 2024

Copy link
Copy Markdown
Collaborator

@nekrich what encryption issues are you running into? Could it be related to the encryption changes I made since the last release?

@lacostej

lacostej commented Feb 9, 2024

Copy link
Copy Markdown
Collaborator

@nekrich so to summarize.

and you would like #21812 to be merged while you fix this one.

The plan sounds OK, but I would really like to know more about the encryption failures you are having, and make sure they are not related to other changes.

@nekrich

nekrich commented Feb 9, 2024

Copy link
Copy Markdown
Contributor Author

@lacostej I'll be able to look only after Feb 20. I'll definitely describe what's wrong.

@rogerluan

rogerluan commented Feb 10, 2024

Copy link
Copy Markdown
Member

I've just merged #21691 — to proceed with this PR, could you rebase the PR, and revert the changes in #21691? So this PR ends up being a "revert the revert + fix the issue" 😅

@lacostej lacostej mentioned this pull request Feb 17, 2024
Closed
4 tasks
@StefanWallin

StefanWallin commented Apr 9, 2024

Copy link
Copy Markdown

Stumbled upon this and just want to let you know we've passed Feb 20 now :)

@bokanechase-digistorm

bokanechase-digistorm commented Apr 29, 2024 •
edited
Loading

Copy link
Copy Markdown

@nekrich looking forward to this feature 😄 🙏

@AliSoftware AliSoftware mentioned this pull request May 15, 2024
Closed
@stherold

stherold commented Sep 6, 2024

Copy link
Copy Markdown
Contributor

Any news here?

@hugo-advizr

hugo-advizr commented Sep 20, 2024

Copy link
Copy Markdown

any plans to merge this one? we need it to be able to automatically renew expired certs. thanks

@sanduluca

sanduluca commented Oct 24, 2024

Copy link
Copy Markdown

Any updates on this ? Can we have the steps that have to be done to renew them manually ?

@gholias

gholias commented Dec 20, 2024

Copy link
Copy Markdown

Any updates on this feature? We really need it

@hugo-advizr

hugo-advizr commented Jan 2, 2025

Copy link
Copy Markdown

@nekrich @lacostej do you have any visibility on when you might have time to merge this one? I completely understand if you don't have availability at the moment, I'm just trying to understand if it's worth investigating and creating a temporary script to automate the certificates renewal (we've hundreds of apps in the match repo, I can't do it manually).
thanks again for all your effort on this!

@gholias

gholias commented Jan 3, 2025

Copy link
Copy Markdown

@hugo-advizr It Seems like you and I are facing exactly the same problem with the certificate's auto-renewal. I would love to brainstorm a solution with you. Do you use twitter or another social network I can use to DM you?

tbodt added a commit to tbodt/fastlane that referenced this pull request Jan 23, 2025
@tbodt
Remove claim that match "automatically repears expired certificates"
bd944d5 
Until fastlane#21809 is merged, it doesn't.
@tbodt tbodt mentioned this pull request Jan 23, 2025
Open
@d-brtn

d-brtn commented Apr 2, 2025

Copy link
Copy Markdown

Any updates on this? This feature would be extremely helpful @nekrich, especially considering functionality was removed in favor of this PR

@MirceaX2Mobile

MirceaX2Mobile commented Jul 3, 2025

Copy link
Copy Markdown

Any update on auto-renewal of expired certificates?

@MattWallaceAzra

MattWallaceAzra commented Feb 10, 2026

Copy link
Copy Markdown

Bump- this would be very helpful to have!

@OdNairy OdNairy mentioned this pull request Jun 25, 2026
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

tool: match

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.