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.

Sign up for GitHub

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

Jump to bottom

Adding configs to allow you to run from local #124

Merged
merged 1 commit into from
Aug 16, 2018
Merged

Adding configs to allow you to run from local #124

merged 1 commit into from
Aug 16, 2018

Conversation

christopherhein
Copy link
Member

Why:

  • This makes development much easier allowing you to pass --kubeconfig
    to a local directory and have it listen to the remote apiserver
  • Caveat! This needs to also be configured with the auth webhook so
    development requires an addressable server

Issue: closes #123

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 23, 2018
@christopherhein christopherhein force-pushed the feature/configs-for-local-development branch from 4864a56 to 1a1bd5e Compare July 23, 2018 21:45
mattlandis
mattlandis reviewed Jul 29, 2018
View reviewed changes
cmd/aws-iam-authenticator/root.go Outdated
@@ -80,9 +80,11 @@ func getConfig() (config.Config, error) {
ClusterID: viper.GetString("clusterID"),
ServerEC2DescribeInstancesRoleARN: viper.GetString("server.ec2DescribeInstancesRoleARN"),
LocalhostPort: viper.GetInt("server.port"),
Localhostname: viper.GetString("server.hostname"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it have to be a localhost? Can I provide another hostname?

Copy link
Member Author

@christopherhein christopherhein Aug 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

No, I was following the LocalhostPort reference, I will change both of those

nckturner
nckturner reviewed Aug 1, 2018
View reviewed changes
cmd/aws-iam-authenticator/init.go Outdated
viper.BindPFlag("server.hostname", initCmd.Flags().Lookup("hostname"))

initCmd.Flags().StringP(
"bind",
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: call the flag addr or address.

@nckturner
Copy link
Contributor

Can you update your commit message? --kubeconfig was part of the larger change, correct?

@christopherhein christopherhein force-pushed the feature/configs-for-local-development branch 2 times, most recently from 7c4dcaa to 46cc47d Compare August 2, 2018 23:56
@christopherhein
Copy link
Member Author

@nckturner yeah, changed both the config and renamed all the components to not have a localhost specificity.

@christopherhein
Adding configs to allow you to run from local
6d35823 
**Why:**

* Makes it possible to issue your certificate and key to a specific
  ipaddress, and hostname making local development easier.

Signed-off-by: Christopher Hein <[email protected]>
@christopherhein christopherhein force-pushed the feature/configs-for-local-development branch from 46cc47d to 6d35823 Compare August 3, 2018 00:22
@nckturner
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 6, 2018
@christopherhein
Copy link
Member Author

Does /lgtm trigger a merge on this or does @mattlandis need to /lgtm too?

@nckturner
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christopherhein, nckturner

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-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 13, 2018
@nckturner nckturner merged commit 62fa370 into kubernetes-sigs:master Aug 16, 2018
@christopherhein christopherhein mentioned this pull request Aug 21, 2018
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattlandis mattlandis mattlandis left review comments

@nckturner nckturner nckturner left review comments

Assignees

@nckturner nckturner

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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow Configuration of the Authenticator from a non-localhost ENV
4 participants
@christopherhein @nckturner @k8s-ci-robot @mattlandis