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

Allow Setting Default Mode In Secret Volumes #452

Merged
merged 1 commit into from
Apr 16, 2019
Merged

Allow Setting Default Mode In Secret Volumes #452

merged 1 commit into from
Apr 16, 2019

Conversation

wesmcouch
Copy link
Contributor

Addressing JENKINS-49641

This change allows you to specify defaultMode for a secretVolume in the podtemplate.

Currently when using SSH private keys in a secret volume, you must specify using YAML in the podTemplate in order to get the correct permissions on the files.

I have tested this it works with and without the defaultMode specified to maintain backwards compatibility.

@wesmcouch wesmcouch closed this Mar 29, 2019
@wesmcouch wesmcouch reopened this Mar 29, 2019
@wesmcouch
Copy link
Contributor Author

@carlossg Let me know if there is anything I can do to improve this, the changes are pretty benign.

@@ -33,21 +33,34 @@
import io.fabric8.kubernetes.api.model.VolumeBuilder;

public class SecretVolume extends PodVolume {
private static final String DEFAULT_MODE = "420";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why 420 ?
the default in kubernetes is 0644 https://kubernetes.io/docs/concepts/configuration/secret/

I would set the default to empty string and then add it to the buildVolume if not empty

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checkout this link under 'Secret files permissions' https://kubernetes.io/docs/concepts/configuration/secret/#using-secrets-as-files-from-a-pod

Using JSON for kubernetes requests does not support Octal notation. 420 maps to 0644. Specifying octal notation in the YAML (without these changes) does not work, you have to use the JSON spec. I haven't dived too deep into the plugin but I am assuming that it uses JSON for requests.

Copy link
Contributor Author

@wesmcouch wesmcouch Apr 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have adjusted the code a bit. The new implementation gets rid of the defaults and only sets the defaultMode in the buildVolume method if that variable is set.

SecretVolumeSource secretVolumeSource = new SecretVolumeSource();
secretVolumeSource.setSecretName(getSecretName());

if (defaultMode != null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use StringUtils.isBlank because the UI will set it to "" IIRC

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect thanks

@wesmcouch
Copy link
Contributor Author

I updated my branch to latest and rebased the commits, it should be ready to go. Thanks!

@carlossg carlossg merged commit cc8f7b3 into jenkinsci:master Apr 16, 2019
@carlossg
Copy link
Contributor

thanks!

@wesmcouch wesmcouch deleted the feature/default-mode-secret-volumes branch April 16, 2019 17:34
@carlossg
Copy link
Contributor

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.

3 participants