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

adds auth import to solve #283 #300

Closed
wants to merge 1 commit into from
Closed

adds auth import to solve #283 #300

wants to merge 1 commit into from

Conversation

stefanhenseler
Copy link
Contributor

This imports the k8s client auth package in order to solve issue #283, allowing auth if auth providers are in use.

@stefanhenseler
Copy link
Contributor Author

I found this: kubernetes/client-go#49. The community decided to remove the import in kubectl by default, because of the considerably big dependency footprint. Are we sure we want to add this import? We can also add documentation and point to the OIDC setup as suggested here: kubernetes/kubernetes#41532. This issue shows that the change (removing the auth package import) caused some projects to have the same problem we have.

an alternative would be to just add the auth/gcp package as they did: boz/kail@89132ac. This would only add support for the gcp provider.

@harshavardhana how do you want to proceed?

@harshavardhana
Copy link
Member

I found this: kubernetes/client-go#49. The community decided to remove the import in kubectl by default, because of the considerably big dependency footprint. Are we sure we want to add this import? We can also add documentation and point to the OIDC setup as suggested here: kubernetes/kubernetes#41532. This issue shows that the change (removing the auth package import) caused some projects to have the same problem we have.

an alternative would be to just add the auth/gcp package as they did: boz/kail@89132ac. This would only add support for the gcp provider.

@harshavardhana how do you want to proceed?

We can do what k8s recommends upstream and document it.

@dvaldivia
Copy link
Collaborator

The tests fail merely because of linting, you can add an annotation so lint skips this line I believe

Running lint check
98
pkg/controller/cluster/main-controller.go:59:2: a blank import should be only in a main or test package, or have a comment justifying it (golint)
99
	_ "k8s.io/client-go/plugin/pkg/client/auth"
100
	^

@jneo8
Copy link

jneo8 commented Oct 5, 2020

@stefanhenseler It work well on GCP. Could you help to add //nolint after line?

@ghost
Copy link

ghost commented Oct 8, 2020

bump this PR please :)

@harshavardhana
Copy link
Member

This is merged closing.

jmontleon pushed a commit to jmontleon/operator that referenced this pull request Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants