Skip to content

Conversation

@deads2k
Copy link
Contributor

@deads2k deads2k commented Feb 22, 2019

The KCM and the admin.kubeconfig both need the complete set of certs
to use to trust the KAS.

This is needed so the kube-apiserver can serve with it's shiny new certs. openshift/cluster-kube-apiserver-operator#282

/assign @abhinavdahiya

Copy link
Contributor

@staebler staebler left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Will the kubelet-client-ca-bundle.crt still be used by anything once it is removed from the kubelet kubeconfig?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will the kubelet-client-ca-bundle.crt still be used by anything once it is removed from the kubelet kubeconfig?

I think it was wired improperly here before (kube-ca for everyone!). It is logically distinct and is for verifying kubelet clients.

@deads2k deads2k force-pushed the kube-apiserver-server-ca branch from 439fa46 to 4131408 Compare February 22, 2019 21:04
@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 22, 2019
@deads2k deads2k force-pushed the kube-apiserver-server-ca branch from 4131408 to f52512c Compare February 22, 2019 21:05
@deads2k
Copy link
Contributor Author

deads2k commented Feb 22, 2019

Please regenerate the asset dependency graph in a separate commit.

https://github.com/openshift/installer/blob/master/docs/design/assetgeneration.md#dependency-graph

done

Copy link
Contributor

Choose a reason for hiding this comment

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

nit:
KAS can use this file to trust the client certificate generated by admin-kubeconfig-signer. just drop kubeapiserver*signer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I deleted these lines. We're trying to create a call bundle to verify the KAS serving cert, not one to verify clients.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I understand now. Sure, I'll make it client ca only. Lgty otherwise? An lgtm and hold so I can get it this weekend?

@abhinavdahiya
Copy link
Contributor

Oh, I understand now. Sure, I'll make it client ca only. Lgty otherwise? An lgtm and hold so I can get it this weekend?

/approve

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, deads2k

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

The pull request process is described here

Details 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 openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 22, 2019
The KCM and the admin.kubeconfig both need the complete set of certs
to use to trust the KAS.
@deads2k deads2k force-pushed the kube-apiserver-server-ca branch from f52512c to 4ae423d Compare February 23, 2019 00:19
@deads2k deads2k added the lgtm Indicates that a PR is ready to be merged. label Feb 23, 2019
@deads2k
Copy link
Contributor Author

deads2k commented Feb 23, 2019

made that client-ca and tagging per comment

@deads2k
Copy link
Contributor Author

deads2k commented Feb 23, 2019

/retest

1 similar comment
@deads2k
Copy link
Contributor Author

deads2k commented Feb 23, 2019

/retest

@deads2k
Copy link
Contributor Author

deads2k commented Feb 23, 2019

/test e2e-aws

@deads2k
Copy link
Contributor Author

deads2k commented Feb 23, 2019

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@deads2k
Copy link
Contributor Author

deads2k commented Feb 23, 2019

aws, aws, aws

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@deads2k
Copy link
Contributor Author

deads2k commented Feb 23, 2019

/test e2e-aws

1 similar comment
@deads2k
Copy link
Contributor Author

deads2k commented Feb 23, 2019

/test e2e-aws

@deads2k
Copy link
Contributor Author

deads2k commented Feb 23, 2019

cadvisor

/retest

@deads2k
Copy link
Contributor Author

deads2k commented Feb 23, 2019

This fixes sa ca.crt bundles. Green, with no overlap to the other AWS one I merged

@deads2k deads2k merged commit 310f685 into openshift:master Feb 23, 2019
@openshift-ci-robot
Copy link
Contributor

@deads2k: The following test failed for commit 4ae423d, say /retest to rerun them:

Test name Details Rerun command
ci/prow/e2e-aws link /test e2e-aws

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.

Details

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants