Skip to content
Snippets Groups Projects

Add support for TLS client authentication

All threads resolved!

Allow gitlab-ci to connect to a gitlab host using TLS client authentication (mutual authentication). Adds configuration and support for using TLS client certificates when using go's TLS transport layer and also sets git enviromental variables for runners.

See also #1291 (closed) and !86 (closed)

Merge request reports

Pipeline #7740617 passed

Pipeline passed for 10d23a03 on sfnelson:feature/tls-client-authentication

Approved by

Merged by avatar (Feb 22, 2025 1:05am UTC)

Merge details

  • Changes merged into master with 351781d7.
  • Did not delete the source branch.

Pipeline #7849750 passed with warnings

Pipeline passed with warnings for 351781d7 on master

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • @sfnelson We should also support client certificates by this command helpers: https://gitlab.com/gitlab-org/gitlab-ci-multi-runner/tree/master/commands/helpers (they are used to upload/download artifacts).

    It means setting: CI_SERVER_TLS_CLIENT_CERT_FILE and CI_SERVER_TLS_CLIENT_KEY_FILE before executing this commands.

    Edited by Kamil Trzciński
  • Nick Thomas Mentioned in merge request !340 (closed)

    Mentioned in merge request !340 (closed)

  • mentioned in issue #1736 (closed)

  • Stephen Nelson added 991 commits

    added 991 commits

    Compare with previous version

  • Stephen Nelson added 1 commit

    added 1 commit

    • 12529a6b - Add support for TLS client authentication

    Compare with previous version

  • I've updated this merge request. @ayufan would you take a look please? It seems like a commonly requested feature, would be great to see it merged.

  • @tmaczukin Could you review it?

  • Kamil Trzciński
  • Stephen Nelson added 75 commits

    added 75 commits

    Compare with previous version

  • Updated for 9.0

  • @tmaczukin I'm currently debugging a potential problem from porting my changes to master. The code is ready for review, but please don't merge it until I confirm that it's ready.

  • Stephen Nelson added 1 commit

    added 1 commit

    • 7c609178 - Add support for TLS client authentication

    Compare with previous version

  • Stephen Nelson added 1 commit

    added 1 commit

    • 67ec679d - Add support for TLS client authentication

    Compare with previous version

  • Done, I had missed a block when porting to 9.0, certificates and builds are working as expected now.

  • @tmaczukin this issue is extremely important to our business. What can I do to keep the process moving?

  • @sfnelson I'll review this today :)

  • @tmaczukin did you get a chance to review this MR? Do you have any comments?

  • Hi @tmaczukin, have you had a chance to look at this yet?

  • @sfnelson Looking on this right now :)

  • @sfnelson Looks really nice :).

    I left few comments. Please fix these and resolve conflicts. Meanwhile I'll try to perform some manual tests using client authentication and this MR :)

  • Tomasz Maczukin mentioned in merge request !86 (closed)

    mentioned in merge request !86 (closed)

  • Tomasz Maczukin changed milestone to %v9.1

    changed milestone to %v9.1

  • Thanks for your comments. I've got some time to look at it now.

  • Stephen Nelson added 77 commits

    added 77 commits

    Compare with previous version

  • Stephen Nelson resolved all discussions

    resolved all discussions

  • Assuming the pipeline passes, I think I've resolved everything you requested. Sorry I didn't have time to get back to it sooner.

  • Stephen Nelson added 1 commit

    added 1 commit

    • a3fcc3dc - Add support for TLS client authentication

    Compare with previous version

  • @tmaczukin ready for review

  • @sfnelson I left two comments

  • Stephen Nelson added 1 commit

    added 1 commit

    • 10d23a03 - Add support for TLS client authentication

    Compare with previous version

  • Tomasz Maczukin resolved all discussions

    resolved all discussions

  • @sfnelson I think all is good now.

    However we didn't have time to install this with RC version on our shared runners to check if this doesn't introduce any regression. And since this MR touches places where communication with GitLab is made, any regression may affect all users - even if they are not using TLS Client Authentication. I don't think that merging this a day before release will be a good idea.

    I'm moving this MR to 9.2. It will be merged at Monday so anyone will be able to install this with Bleeding Edge version which will be basically 9.2 with this one change.

    Thank you for your awesome work on this MR! :smile:

  • Tomasz Maczukin changed milestone to %v9.2

    changed milestone to %v9.2

  • It's been almost a year, another few days won't hurt. Looking forward to seeing it merged.

  • Tomasz Maczukin approved this merge request

    approved this merge request

  • Tomasz Maczukin mentioned in commit 351781d7

    mentioned in commit 351781d7

  • mentioned in issue #1291 (closed)

  • @tmaczukin awesome, thanks

  • mentioned in issue #2570 (closed)

  • Please register or sign in to reply
    Loading