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 AWS ECR support #920

Closed
wants to merge 7 commits into from
Closed

Conversation

chicocvenancio
Copy link
Contributor

Starting the PR, I'll push a few more commits before it is ready for review.

FIX #705

@chicocvenancio chicocvenancio marked this pull request as ready for review August 28, 2019 18:55
@betatim
Copy link
Member

betatim commented Nov 8, 2019

Conflicts need resolving and the test failures checking out. I restarted the build and it failed again so I think this might be a real failure.

Is there a way to avoid having to expand the RBAC rights? If we can avoid giving more permissions that would be a "good thing (tm)". So I thought I'd raise it to see what we can come up with.

What does it mean for people upgrading if we change the RBAC permissions? I have a vague memory that there is a difference between creating and editing permission.

@chicocvenancio
Copy link
Contributor Author

I'll try to make the config more robust, at this point it is expecting AWS specific config to be there, and since it is not for the CI tests it is failing.

Is there a way to avoid having to expand the RBAC rights? If we can avoid giving more permissions that would be a "good thing (tm)". So I thought I'd raise it to see what we can come up with.
I can't think of a straightforward way of dropping added rights entirely. ECR login tokens expire in 12 hours, right now the tokens are set both in DockerRegistry.password and as a secret binder-push-secret, both need to be set for pushing and pulling images to work. One option to restrict RBAC a little further is to specify the binder-push-secret in the rule, so it only applies to it.

What does it mean for people upgrading if we change the RBAC permissions? I have a vague memory that there is a difference between creating and editing permission.
We're using Pulumi to deploy Binderhub, so I'm not completely sure if helm+tiller does something weird here. I wouldn't expect it to, but I can take a look and test it out.

@ivan-gomes
Copy link
Contributor

@chicocvenancio anything I can help with to get this to the finish line?

@meeseeksmachine
Copy link

This pull request has been mentioned on Jupyter Community Forum. There might be relevant details there:

https://discourse.jupyter.org/t/binder-jupyterhub-activity-round-up-week-1/2704/1

@consideRatio
Copy link
Member

Hey @chicocvenancio, @ivan-gomes and @betatim! I'm going through various PRs hoping to help them resolve one way or another or perhaps get an updated description about the current status of things.

I'm closing this in favor of the PR building on this with additional commits: #1055.

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.

AWS ECR registry for BinderHub deployment
5 participants