search

drone-plugins / drone-s3-cache

Caches build artifacts to S3 compatible storage backends
http://plugins.drone.io/drone-plugins/drone-s3-cache
Apache License 2.0
28 stars 30 forks source link

Add loading of ca certificates to plugin #34

Closed kneal closed 6 years ago

kneal commented 6 years ago

Using internal server which requires certificates to authenticate. Tested internally and works just fine.

cc @jbrockopp @jmccann

donny-dont commented 6 years ago

@jbrockopp @jmccann I don't think those are standard variables that users of AWS expect like they would with AWS_ACCESS_KEY_ID so lets not make more environment variables.

jbrockopp commented 6 years ago

@donny-dont @jmccann that sounds good. We weren't sure so we figured we would just add them initially and remove them later if its deemed a problem. We'll make a commit to remove those and ping you after its done.

kneal commented 6 years ago

@jmccann @donny-dont Updated

tboerger commented 6 years ago

Instead of doing it like that or should be much better to include your custom ca certificates into all containers... Drone got an env variable to add custom volumes to all containers... That's how I'm doing that. Otherwise we end up with adding these options to various plugins...

bradrydzewski commented 6 years ago

yep, I would instead recommend using the global volume feature to mount certs into containers.

edit: you can pass the DRONE_VOLUME=/path/on/host:/path/in/container environment variable to your drone server. This value will be sent down to all agents, and all volumes will be mounted into all containers.

jmccann commented 6 years ago

So then the /path/on/host file specified in DRONE_VOLUME on the server would actually need to exist on all the agents, right?

tboerger commented 6 years ago

That's right, it doesn't need to exist on the server, only on the agents. But if you are using a custom root certificate somebody can expect that it is anyway present on the host.

donny-dont commented 6 years ago

@jmccann can we remove this code now?

jmccann commented 6 years ago

@donny-dont To be honest I'm not sure. @jbrockopp would be better suited to answer that now.

tboerger commented 6 years ago

IMHO this had never been added to the plugin. This is not a plugin logic...

jbrockopp commented 6 years ago

@donny-dont @tboerger you can remove the logic if you feel it doesn't belong.

@bradrydzewski is their anyway to "turn off" the DRONE_VOLUME for a specific step?

The scenario I'm concerned with is customers needing to inject their own CA cert for a specific step and being unable to since the volume would be mounted in.

jbrockopp commented 6 years ago

@tboerger @donny-dont do either of you have thoughts on the above?

... is their anyway to "turn off" the DRONE_VOLUME for a specific step?

The scenario I'm concerned with is customers needing to inject their own CA cert for a specific step and being unable to since the volume would be mounted in.

jbrockopp commented 6 years ago

@bradrydzewski @tboerger @donny-dont ping to see if you have thoughts on ^^.

I understand the wanting to remove this feature but if there isn't a way to "turn off" the DRONE_VOLUME feature for a step then I would argue that they don't provide the same functionality and this feature should remain in the plugin.

Thoughts?