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

UPSTREAM: <carry>: don't add service-ca to SA tokens #24393

Closed
wants to merge 1 commit into from

Conversation

stlaz
Copy link
Member

@stlaz stlaz commented Jan 14, 2020

This PR is here for 4.5 where the /var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt will be remove as announced in 4.1 release notes.

openshift/openshift-docs#18426 (comment)

/cc @deads2k @marun

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 14, 2020
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. vendor-update Touching vendor dir or related files labels Jan 14, 2020
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: stlaz
To complete the pull request process, please assign smarterclayton
You can assign the PR to them by writing /assign @smarterclayton in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot
Copy link

@stlaz: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-gcp cde8e32 link /test e2e-gcp
ci/prow/e2e-gcp-upgrade cde8e32 link /test e2e-gcp-upgrade
ci/prow/e2e-cmd cde8e32 link /test e2e-cmd
ci/prow/e2e-aws-fips cde8e32 link /test e2e-aws-fips
ci/prow/unit cde8e32 link /test unit

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 14, 2020
@marun
Copy link
Contributor

marun commented May 7, 2020

@stlaz As per this bz this PR is deferred to 4.6.

Edit: Would it make sense to remove UPSTREAM: <carry>: kube-controller-manager: add service serving cert signer to token controller as well?

@marun
Copy link
Contributor

marun commented May 7, 2020

/hold

@stlaz
Copy link
Member Author

stlaz commented May 25, 2020

@stlaz As per this bz this PR is deferred to 4.6.

Edit: Would it make sense to remove UPSTREAM: <carry>: kube-controller-manager: add service serving cert signer to token controller as well?

Yes, we should do that as well to clear this all away

@openshift-bot
Copy link
Contributor

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci-robot openshift-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 24, 2020
@openshift-bot
Copy link
Contributor

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci-robot
Copy link

@openshift-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@marun
Copy link
Contributor

marun commented Jul 24, 2020

afaik we can't disable this since we can't prove it won't negatively impact customers.

@slaskawi
Copy link

@stlaz @marun Keycloak Operator uses the service-ca.crt, so I'm very interested if this PR got descoped for good or it was just delayed to another release?

@stlaz
Copy link
Member Author

stlaz commented Aug 10, 2020

@slaskawi it did, but I hope we'll get to do it one day because the amount of unused CA certs that take up space in etcd for no reason is ridiculous.

You can just do https://docs.openshift.com/container-platform/4.5/security/certificates/service-serving-certificate.html#add-service-certificate-configmap_service-serving-certificate and mount the CM to your pod instead.

@slaskawi
Copy link

Thanks for the hint @stlaz !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. vendor-update Touching vendor dir or related files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants