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

Write SSH keys in default encoding #146

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ricedavida
Copy link

The code change will take, if provided, the user property setting of an ssh key's file encoding,
and it will set it on the remote agent's computer. If left unset, it will use the default of UTF-8.

This feature is needed for edge systems that don't use UTF-8 natively, like zOS(the mainframe). So, to allow the
ssh keyfiles to be generated correctly, we can specify the encoding type.

This has been manually tested on a jenkins/zOS, and works as intended.

    This feature is needed for edge systems that don't use UTF-8
    natively, like z/OS.
@jglick
Copy link
Member

jglick commented Oct 18, 2021

Can this not just be set to ASCII?

@ricedavida
Copy link
Author

Unfortunately, no. zOS natively uses IBM-1047 (EBCDIC) code pages. This seems to be the only way that this could be addressed for that operating system.

@jglick
Copy link
Member

jglick commented Oct 18, 2021

So then the correct fix should be to use the agent JVM’s default encoding? Which would usually be UTF-8, occasionally some ASCII superset on Windows machines for example, and EBCDIC on z/OS?

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Can you verify that the default encoding approach indeed works on z/OS?

Comment on lines 106 to 108
keyFile.write(contents.toString(), computer.getDefaultCharset());

keyFile.chmod(0400);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
keyFile.write(contents.toString(), computer.getDefaultCharset());
keyFile.chmod(0400);
keyFile.write(contents.toString(), computer.getDefaultCharset());
keyFile.chmod(0400);

(tabs → spaces for consistency)

Comment on lines 105 to 106
Computer computer = keyFile.toComputer();
keyFile.write(contents.toString(), computer.getDefaultCharset());
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Computer computer = keyFile.toComputer();
keyFile.write(contents.toString(), computer.getDefaultCharset());
keyFile.write(contents.toString(), null);

as per https://javadoc.jenkins.io/hudson/FilePath.html#write-java.lang.String-java.lang.String-

@jglick jglick changed the title Enable ssh keys to be written with user provided property Write SSH keys in default encoding Oct 21, 2021
@jglick jglick added the bug label Oct 21, 2021
@jglick
Copy link
Member

jglick commented Oct 21, 2021

BTW what will the impact be on z/OS of openjdk/jdk#4733 whenever that is ported?

@ricedavida
Copy link
Author

sorry, I am in the process of validating this now, and will clean up the pull request once everything is validated.

@jglick jglick marked this pull request as draft January 5, 2022 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants