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

add support for rbac v1 API endpoint #6

Merged
merged 1 commit into from
Jan 31, 2018

Conversation

n0n0x
Copy link

@n0n0x n0n0x commented Dec 15, 2017

As of kubernetes 1.8 RBAC is now stable and the endpoint has changed to rbac.authorization.k8s.io/v1 . I have kept compatibility for older k8s versions.

This is my first PR, let me know if something is missing!

List of PRs related to this:

controller: #6
router: teamhephy/router#5
monitor: teamhephy/monitor#1
fluentd: teamhephy/fluentd#2
builder: teamhephy/builder#1

kubernetes >=1.8 moved rbac to v1
@Cryptophobia
Copy link
Member

Cryptophobia commented Dec 15, 2017

Thank you for the contribution! Looks good @n0n0x . We are currently still setting up e2e testing so I will need to test it out manually on 1.8 and will merge as soon as I can.

@Cryptophobia Cryptophobia self-requested a review December 15, 2017 20:40
@Cryptophobia Cryptophobia self-assigned this Dec 15, 2017
@tmedford
Copy link

Thanks so much, we are looking to move the direction. Be sure to check the updates for router.

@kingdonb
Copy link
Member

Assuming this passes testing, it should ideally get merged simultaneously with four other components that also have RBAC roles and bindings. As long as it all goes into the same release

I looked at E2E testing this weekend and did not get it done, but I think I will be trying this against GKE and Minikube first. I don't know if Minikube alone can support the E2E tests on my machines. (The specs for the test cluster are larger than any machines I have currently at my disposal, but I have a number of free GKE credits, so I should be able to run the suite a few times at no cost.)

I'm going to try to execute the E2E tests without the whole cluster leasing thing at first, as I can't afford to keep multiple test clusters running continuously throughout the day when they are not being used... I'm hoping it does not take more than about two hours for the full suite to run with an appropriately sized cluster and I can get a bead on the cost to test a PR against a single cloud type.

Running the E2E suite for the first time will probably go smoother (or get done faster) if someone else who has already run the suite before is available for a pep-talk.

@n0n0x
Copy link
Author

n0n0x commented Dec 18, 2017

Awesome, I'll get into adapting the other components that use RBAC, @kingdonb besides controller and router which are the other two components?

@n0n0x
Copy link
Author

n0n0x commented Dec 18, 2017

Done, Did a quick search on the organization and found all the other components, here is a list of PRs:

controller: #6
router: teamhephy/router#5
monitor: teamhephy/monitor#1
fluentd: teamhephy/fluentd#2
builder: teamhephy/builder#1

Let me know if any other component is missing!

@kingdonb
Copy link
Member

That is the same list of 5 that I came up with when I grepped. Thank you @n0n0x !

@kingdonb
Copy link
Member

That's interesting... teamhephy/builder#1 CI failed

@n0n0x
Copy link
Author

n0n0x commented Dec 18, 2017

Yep, according to travi's job log, it is unrelated to my PR. The .travis.yml seems to be missing make bootstrap which installs dependencies.

Here's a link to the previous .travis.yml from deis and the new one

I can maybe copy it and open a new PR and see if it works, but It might fetch things from deis repo

@n0n0x
Copy link
Author

n0n0x commented Dec 19, 2017

I made a new PR (teamhephy/builder#2) where CI passes, but I am skeptic of the actual results 😝

@Cryptophobia Cryptophobia merged commit 2bf9a23 into teamhephy:master Jan 31, 2018
duanhongyi added a commit to duanhongyi/controller that referenced this pull request Jan 3, 2021
chore(Certificate):change cluster-issuer location
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