rules_docker icon indicating copy to clipboard operation
rules_docker copied to clipboard

Add insecure repository support

Open fahhem opened this issue 5 years ago • 8 comments

Local registries, such as microk8s, don't have SSL setup.

This PR just adds the ability to specify to push to an insecure repository. I'm not sure if that's the intended design pattern for this, so I'm open to suggestions!

fahhem avatar Jan 28 '20 21:01 fahhem

/assign @smukherj1

fahhem avatar Jan 28 '20 21:01 fahhem

This might also address #1245

fahhem avatar Jan 28 '20 21:01 fahhem

Any update on this?

jake-arkinstall avatar Jun 22 '20 14:06 jake-arkinstall

@fahhem will you finish this PR or can I post mine? For our production, this is a very important feature in rules_docker.

asv avatar Jan 25 '21 06:01 asv

I don't have a publicly-available insecure repository available to add a test against. If you have a better PR, go ahead and post it, I'm not attached to this PR in any way. I was simply sharing what I had to do to get a on-machine docker repository to work.

fahhem avatar Jan 25 '21 16:01 fahhem

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fahhem To complete the pull request process, please assign smukherj1 after the PR has been reviewed. You can assign the PR to them by writing /assign @smukherj1 in a comment when ready.

The full list of commands accepted by this bot can be found 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 avatar Feb 03 '21 21:02 k8s-ci-robot

I've rebased onto master. I don't know the test infra here, so I added a "test" that is probably wrong. :shrug: if anyone else has a better test or at least a suggestion on how to write one, I'm happy to swap mine out for it.

fahhem avatar Feb 03 '21 22:02 fahhem

We are very interested in having this merged in. We are also dependent on a registry which is insecure and are having to write lots of workarounds without this change.

cfife-btig avatar Aug 22 '22 16:08 cfife-btig

Rebased again. This time the tests are failing for the same reason the master tests are failing (something about an unsupported version of ubuntu? ubuntu2104 vs the mentioned ubuntu1804)

fahhem avatar Sep 26 '22 06:09 fahhem

@uhthomas any chance this can be reviewed? It's a pretty small PR :)

fahhem avatar Oct 18 '22 01:10 fahhem

@linzhp any idea on what I should do about the required-but-failing CI step? Or will you be able to merge it anyway?

fahhem avatar Oct 18 '22 04:10 fahhem

Oh, and thank you!

fahhem avatar Oct 18 '22 04:10 fahhem

It doesn't allow me to merge without CI passing

linzhp avatar Oct 18 '22 04:10 linzhp

I'm just happy to see some progress is on this MR :')

cfife-btig avatar Oct 18 '22 16:10 cfife-btig

Same here :D Thanks @uhthomas, I ran gofmt and it had a couple more changes than around my code, but it was all whitespace stuff.

fahhem avatar Oct 18 '22 18:10 fahhem

sorry for the noise, github's reviewer UI looks... strange. Anyone consider reviewable.io?

fahhem avatar Oct 18 '22 23:10 fahhem

@uhthomas sorry to bother!

fahhem avatar Oct 29 '22 01:10 fahhem

I don't mean to keep pecking either. But my team is also really interested in getting this merged in as well. I know that the CI is still being blocked but it sorta sounds like it just might be some sort of noise and not anything legitimate. Although of course I also have no visibility.

cfife-btig avatar Nov 03 '22 22:11 cfife-btig

Looks okay, we'll need to understand why CI isn't happy and then we can merge.

uhthomas avatar Nov 03 '22 23:11 uhthomas

All PR's are blocked until CI is fixed? Is anyone at bazel working on fixing the CI?

It's a lot easier to fix buildkite stuff when you have access to the servers, but I can take a stab at it still.

fahhem avatar Nov 04 '22 02:11 fahhem

Sorry for the delay. I just fixed CI (#2178)

linzhp avatar Nov 06 '22 17:11 linzhp

Thank you @linzhp I fixed the tests too, so this PR should be ready to go!

fahhem avatar Nov 08 '22 21:11 fahhem

🎉🎉🎉

cfife-btig avatar Nov 08 '22 22:11 cfife-btig

Any idea when will this be released? Waiting on this fix.

yc185050 avatar Jan 12 '23 15:01 yc185050

@yc185050 I recommend opening a new issue to request a release. Personally, I'm using master at that merged commit as my company's release while we wait for an official release

fahhem avatar Jan 17 '23 06:01 fahhem