Metaswitch / clearwater-docker

Docker integration for Project Clearwater
Other
41 stars 64 forks source link

added option to authenticate with registry #88

Closed mdavis-xyz closed 6 years ago

mdavis-xyz commented 6 years ago

If you use a private container registry, you need to add a parameter to each yaml file. Doing that manually is a pain, so I added it to k8s-gencfg.

Since we're not using a full template library, I found it easiest to leave the new parameter in the yaml files even if you don't need it, and just set it to ~ (yaml null). I don't have a public registry to test with, so it would be good if someone else can double check that ~ works when you don't need to authenticate. It is valid yaml, so I expect it to work.

mdavis-xyz commented 6 years ago

Ok @johadalin is that all good to merge now? (Pending your own tests with no authentication)

johadalin commented 6 years ago

Looks like this all works fine on my side, and i like the way you've formatted the templating part now 👍

From the tech side i think this is all good to merge. As i commented on a separate pull request though, we need to sort out the contributors agreement before we're able to merge this down to master. Legal licencing stuff is always a bit of a pain, but if you email the address i pointed at (linked at http://www.projectclearwater.org/community/), we should hopefully be able to sort that out quickly, and then can merge this down.

Thanks again for pushing these changes over :) nice addition to the setup

mdavis-xyz commented 6 years ago

Yep, I've already sent an email to that address. I'm just awaiting the reply.

johadalin commented 6 years ago

Hey @mlda065 . Any further news on the agreement? I believe you should have got some stuff sent over; both corporate and individual ones in this case, right?

mdavis-xyz commented 6 years ago

I just submitted the signed agreements via email a minute ago.

johadalin commented 6 years ago

Excellent, all confirmed on this side too, so i'm going to go ahead and merge it down 👍