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

apiserver: document how to run sample-apiserver standalone outside the cluster #55476

Merged

Conversation

sttts
Copy link
Contributor

@sttts sttts commented Nov 10, 2017

This PR documents how to run the sample-apiserver outside of a cluster for development.

tl/dr: local client CA with system:masters group membership. Then authorization is skipped.

@k8s-ci-robot k8s-ci-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 10, 2017
@sttts sttts assigned deads2k and unassigned lavalamp and liggitt Nov 10, 2017
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 10, 2017
@sttts sttts force-pushed the sttts-sample-apiserver-standalone branch from 06014f4 to 770bc0c Compare November 10, 2017 12:24
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 10, 2017
@sttts sttts added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Nov 10, 2017
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Nov 10, 2017
@@ -375,6 +375,10 @@ func (s *DelegatingAuthenticationOptions) getClientConfig() (*rest.Config, error
}

func (s *DelegatingAuthenticationOptions) newTokenAccessReview() (authenticationclient.TokenReviewInterface, error) {
if len(s.RequestHeader.ClientCAFile) > 0 || s.SkipInClusterLookup {
Copy link
Contributor

Choose a reason for hiding this comment

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

I can have a clientCAFile and a tokenaccessreview.

@@ -35,6 +35,9 @@ type DelegatingAuthorizationOptions struct {
// SubjectAccessReview.authorization.k8s.io endpoint for checking tokens.
RemoteKubeConfigFile string

// DisableAuthorization is used to explicitly disable any authorization, mostly for development purposes.
DisableAuthorization bool
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would I want this lever? What's hard about doing development with a pod and image?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we want to extract openapi from a server, i.e. just launch the server, do one curl and terminate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and about pod and image: people want to run an apiserver inside an idea. a multi-process setup is far too complicated. Am happy to call this option explicitly "dev-only" or something like that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Am happy to call this option explicitly "dev-only" or something like that.

Thing is, I really don't even want devs using it. Lack of consideration for security has produced sins we're still paying for after 3 years. What if you made it system:masters only and used a client cert. That gives you the single file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it starts up without a config, but only accepts system:masters, that sounds fine, works for development, but any non-dev use-case immediately need a secure setup. Is this was what you mean? If not, please elaborate.

if err != nil {
return err
if s.DisableAuthorization {
c.Authorizer = authorizerfactory.NewAlwaysAllowAuthorizer()
Copy link
Contributor

Choose a reason for hiding this comment

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

return early here, then no else.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@tamalsaha
Copy link
Member

Thanks @sttts ! This will be very handy for dev setup and openapi generation for non-GO clients.

@deads2k
Copy link
Contributor

deads2k commented Nov 10, 2017

we want to extract openapi from a server, i.e. just launch the server, do one curl and terminate.

This seems like the wrong way to go about that. I would instead try to find a way to get openapi docs without launching a server at all.

@tamalsaha
Copy link
Member

@deads2k , do you how to get the openapi doc without any api server? May be something like that can be added to code-generators.

@sttts
Copy link
Contributor Author

sttts commented Nov 10, 2017

This seems like the wrong way to go about that. I would instead try to find a way to get openapi docs without launching a server at all.

I don't disagree, but I fear this means a huge refactoring of our openapi code.

@sttts sttts force-pushed the sttts-sample-apiserver-standalone branch from 770bc0c to 2464fe5 Compare November 14, 2017 09:27
@sttts
Copy link
Contributor Author

sttts commented Nov 14, 2017

@deads2k updated, only granting access to group system:master using client certs.

@sttts sttts force-pushed the sttts-sample-apiserver-standalone branch from 2464fe5 to f703bc2 Compare November 14, 2017 12:44
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 14, 2017
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 15, 2017
@sttts sttts force-pushed the sttts-sample-apiserver-standalone branch 2 times, most recently from 582508c to 64b50c8 Compare November 15, 2017 12:58
@sttts sttts force-pushed the sttts-sample-apiserver-standalone branch from 48dcdb7 to d22ec24 Compare April 26, 2018 10:22
@kubernetes kubernetes deleted a comment from k8s-github-robot Apr 26, 2018
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 26, 2018
@sttts sttts force-pushed the sttts-sample-apiserver-standalone branch 5 times, most recently from 1ea8307 to 1c8d56a Compare April 27, 2018 07:35
@k8s-ci-robot
Copy link
Contributor

@sttts: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

@k8s-ci-robot k8s-ci-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Apr 27, 2018
@sttts sttts changed the title apiserver: allow sample-apiserver to be run standalone for development apiserver: document how to run sample-apiserver standalone outside the cluster Apr 27, 2018
@sttts
Copy link
Contributor Author

sttts commented Apr 27, 2018

@deads2k @liggitt this is now docs only. ptal.

@sttts sttts force-pushed the sttts-sample-apiserver-standalone branch from 1c8d56a to 9222aa6 Compare April 27, 2018 07:39
@sttts sttts added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Apr 27, 2018
@sttts sttts force-pushed the sttts-sample-apiserver-standalone branch from 9222aa6 to c14fa27 Compare April 27, 2018 07:41
Copy link
Member

@xmudrii xmudrii left a comment

Choose a reason for hiding this comment

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

LGTM. Can confirm this works for local development.

@sttts sttts force-pushed the sttts-sample-apiserver-standalone branch from c14fa27 to 81b0198 Compare April 27, 2018 07:42
@sttts
Copy link
Contributor Author

sttts commented Apr 27, 2018

/retest

@sttts sttts force-pushed the sttts-sample-apiserver-standalone branch from 81b0198 to ad21a4a Compare April 27, 2018 12:58
@sttts
Copy link
Contributor Author

sttts commented May 7, 2018

@deads2k lgty?

@deads2k
Copy link
Contributor

deads2k commented May 7, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 7, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described 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

@k8s-github-robot
Copy link

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 239d612 into kubernetes:master May 7, 2018
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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants