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.

Sign up for GitHub

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

Jump to bottom

persist core.sshCommand for submodules #184

Merged
merged 3 commits into from Mar 12, 2020
Merged

persist core.sshCommand for submodules #184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
518a867
persist core.sshCommand for submodules
ericsciple Mar 11, 2020
2a15e11
update verbiage; add comments
ericsciple Mar 12, 2020
95471c2
fail when submodules or ssh-key and fallback to REST API
ericsciple Mar 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions README.md
View file
Open in desktop
Expand Up @@ -49,19 +49,19 @@ Refer [here](https://github.com/actions/checkout/blob/v1/README.md) for previous
# with the local git config, which enables your scripts to run authenticated git
# commands. The post-job step removes the PAT.
#
# We recommend creating a service account with the least permissions necessary.
# Also when generating a new PAT, select the least scopes necessary.
# We recommend using a service account with the least permissions necessary. Also
# when generating a new PAT, select the least scopes necessary.
#
# [Learn more about creating and using encrypted secrets](https://help.github.com/en/actions/automating-your-workflow-with-github-actions/creating-and-using-encrypted-secrets)
#
# Default: ${{ github.token }}
token: ''

# SSH key used to fetch the repository. SSH key is configured with the local git
# config, which enables your scripts to run authenticated git commands. The
# SSH key used to fetch the repository. The SSH key is configured with the local
# git config, which enables your scripts to run authenticated git commands. The
# post-job step removes the SSH key.
#
# We recommend creating a service account with the least permissions necessary.
# We recommend using a service account with the least permissions necessary.
#
# [Learn more about creating and using encrypted secrets](https://help.github.com/en/actions/automating-your-workflow-with-github-actions/creating-and-using-encrypted-secrets)
ssh-key: ''
Expand Down
86 changes: 32 additions & 54 deletions __test__/git-auth-helper.test.ts
View file
Open in desktop
Expand Up @@ -320,6 +320,8 @@ describe('git-auth-helper tests', () => {
).toString()
expect(actualSshKeyContent).toBe(settings.sshKey + '\n')
if (!isWindows) {
// Assert read/write for user, not group or others.
// Otherwise SSH client will error.
expect((await fs.promises.stat(actualSshKeyPath)).mode & 0o777).toBe(
0o600
)
Expand Down Expand Up @@ -437,15 +439,16 @@ describe('git-auth-helper tests', () => {
}
)

const configureSubmoduleAuth_configuresTokenWhenPersistCredentialsTrueAndSshKeyNotSet =
'configureSubmoduleAuth configures token when persist credentials true and SSH key not set'
const configureSubmoduleAuth_configuresSubmodulesWhenPersistCredentialsFalseAndSshKeyNotSet =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

some of the tests for configureSubmoduleAuth were redundant so i collapsed based on input combinations:

  • persist-credentials: false, ssh-key: ''
  • persist-credentials: false, ssh-key: 'set'
  • persist-credentials: true, ssh-key: ''
  • persist-credentials: true, ssh-key: 'set'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Each test just tests the submoduleForeach calls so it made sense to collapse.

'configureSubmoduleAuth configures submodules when persist credentials false and SSH key not set'
it(
configureSubmoduleAuth_configuresTokenWhenPersistCredentialsTrueAndSshKeyNotSet,
configureSubmoduleAuth_configuresSubmodulesWhenPersistCredentialsFalseAndSshKeyNotSet,
async () => {
// Arrange
await setup(
configureSubmoduleAuth_configuresTokenWhenPersistCredentialsTrueAndSshKeyNotSet
configureSubmoduleAuth_configuresSubmodulesWhenPersistCredentialsFalseAndSshKeyNotSet
)
settings.persistCredentials = false
settings.sshKey = ''
const authHelper = gitAuthHelper.createAuthHelper(git, settings)
await authHelper.configureAuth()
Expand All @@ -456,31 +459,30 @@ describe('git-auth-helper tests', () => {
await authHelper.configureSubmoduleAuth()

// Assert
expect(mockSubmoduleForeach).toHaveBeenCalledTimes(3)
expect(mockSubmoduleForeach.mock.calls[0][0]).toMatch(
expect(mockSubmoduleForeach).toBeCalledTimes(1)
expect(mockSubmoduleForeach.mock.calls[0][0] as string).toMatch(
/unset-all.*insteadOf/
)
expect(mockSubmoduleForeach.mock.calls[1][0]).toMatch(/http.*extraheader/)
expect(mockSubmoduleForeach.mock.calls[2][0]).toMatch(/url.*insteadOf/)
}
)

const configureSubmoduleAuth_configuresTokenWhenPersistCredentialsTrueAndSshKeySet =
'configureSubmoduleAuth configures token when persist credentials true and SSH key set'
const configureSubmoduleAuth_configuresSubmodulesWhenPersistCredentialsFalseAndSshKeySet =
'configureSubmoduleAuth configures submodules when persist credentials false and SSH key set'
it(
configureSubmoduleAuth_configuresTokenWhenPersistCredentialsTrueAndSshKeySet,
configureSubmoduleAuth_configuresSubmodulesWhenPersistCredentialsFalseAndSshKeySet,
async () => {
if (!sshPath) {
process.stdout.write(
`Skipped test "${configureSubmoduleAuth_configuresTokenWhenPersistCredentialsTrueAndSshKeySet}". Executable 'ssh' not found in the PATH.\n`
`Skipped test "${configureSubmoduleAuth_configuresSubmodulesWhenPersistCredentialsFalseAndSshKeySet}". Executable 'ssh' not found in the PATH.\n`
)
return
}

// Arrange
await setup(
configureSubmoduleAuth_configuresTokenWhenPersistCredentialsTrueAndSshKeySet
configureSubmoduleAuth_configuresSubmodulesWhenPersistCredentialsFalseAndSshKeySet
)
settings.persistCredentials = false
const authHelper = gitAuthHelper.createAuthHelper(git, settings)
await authHelper.configureAuth()
const mockSubmoduleForeach = git.submoduleForeach as jest.Mock<any, any>
Expand All @@ -490,24 +492,23 @@ describe('git-auth-helper tests', () => {
await authHelper.configureSubmoduleAuth()

// Assert
expect(mockSubmoduleForeach).toHaveBeenCalledTimes(2)
expect(mockSubmoduleForeach).toHaveBeenCalledTimes(1)
expect(mockSubmoduleForeach.mock.calls[0][0]).toMatch(
/unset-all.*insteadOf/
)
expect(mockSubmoduleForeach.mock.calls[1][0]).toMatch(/http.*extraheader/)
}
)

const configureSubmoduleAuth_doesNotConfigureTokenWhenPersistCredentialsFalse =
'configureSubmoduleAuth does not configure token when persist credentials false'
const configureSubmoduleAuth_configuresSubmodulesWhenPersistCredentialsTrueAndSshKeyNotSet =
'configureSubmoduleAuth configures submodules when persist credentials true and SSH key not set'
it(
configureSubmoduleAuth_doesNotConfigureTokenWhenPersistCredentialsFalse,
configureSubmoduleAuth_configuresSubmodulesWhenPersistCredentialsTrueAndSshKeyNotSet,
async () => {
// Arrange
await setup(
configureSubmoduleAuth_doesNotConfigureTokenWhenPersistCredentialsFalse
configureSubmoduleAuth_configuresSubmodulesWhenPersistCredentialsTrueAndSshKeyNotSet
)
settings.persistCredentials = false
settings.sshKey = ''
const authHelper = gitAuthHelper.createAuthHelper(git, settings)
await authHelper.configureAuth()
const mockSubmoduleForeach = git.submoduleForeach as jest.Mock<any, any>
Expand All @@ -517,28 +518,30 @@ describe('git-auth-helper tests', () => {
await authHelper.configureSubmoduleAuth()

// Assert
expect(mockSubmoduleForeach).toBeCalledTimes(1)
expect(mockSubmoduleForeach.mock.calls[0][0] as string).toMatch(
expect(mockSubmoduleForeach).toHaveBeenCalledTimes(3)
expect(mockSubmoduleForeach.mock.calls[0][0]).toMatch(
/unset-all.*insteadOf/
)
expect(mockSubmoduleForeach.mock.calls[1][0]).toMatch(/http.*extraheader/)
expect(mockSubmoduleForeach.mock.calls[2][0]).toMatch(/url.*insteadOf/)
}
)

const configureSubmoduleAuth_doesNotConfigureUrlInsteadOfWhenPersistCredentialsTrueAndSshKeySet =
'configureSubmoduleAuth does not configure URL insteadOf when persist credentials true and SSH key set'
const configureSubmoduleAuth_configuresSubmodulesWhenPersistCredentialsTrueAndSshKeySet =
'configureSubmoduleAuth configures submodules when persist credentials true and SSH key set'
it(
configureSubmoduleAuth_doesNotConfigureUrlInsteadOfWhenPersistCredentialsTrueAndSshKeySet,
configureSubmoduleAuth_configuresSubmodulesWhenPersistCredentialsTrueAndSshKeySet,
async () => {
if (!sshPath) {
process.stdout.write(
`Skipped test "${configureSubmoduleAuth_doesNotConfigureUrlInsteadOfWhenPersistCredentialsTrueAndSshKeySet}". Executable 'ssh' not found in the PATH.\n`
`Skipped test "${configureSubmoduleAuth_configuresSubmodulesWhenPersistCredentialsTrueAndSshKeySet}". Executable 'ssh' not found in the PATH.\n`
)
return
}

// Arrange
await setup(
configureSubmoduleAuth_doesNotConfigureUrlInsteadOfWhenPersistCredentialsTrueAndSshKeySet
configureSubmoduleAuth_configuresSubmodulesWhenPersistCredentialsTrueAndSshKeySet
)
const authHelper = gitAuthHelper.createAuthHelper(git, settings)
await authHelper.configureAuth()
Expand All @@ -549,37 +552,12 @@ describe('git-auth-helper tests', () => {
await authHelper.configureSubmoduleAuth()

// Assert
expect(mockSubmoduleForeach).toHaveBeenCalledTimes(2)
expect(mockSubmoduleForeach).toHaveBeenCalledTimes(3)
expect(mockSubmoduleForeach.mock.calls[0][0]).toMatch(
/unset-all.*insteadOf/
)
expect(mockSubmoduleForeach.mock.calls[1][0]).toMatch(/http.*extraheader/)
}
)

const configureSubmoduleAuth_removesUrlInsteadOfWhenPersistCredentialsFalse =
'configureSubmoduleAuth removes URL insteadOf when persist credentials false'
it(
configureSubmoduleAuth_removesUrlInsteadOfWhenPersistCredentialsFalse,
async () => {
// Arrange
await setup(
configureSubmoduleAuth_removesUrlInsteadOfWhenPersistCredentialsFalse
)
settings.persistCredentials = false
const authHelper = gitAuthHelper.createAuthHelper(git, settings)
await authHelper.configureAuth()
const mockSubmoduleForeach = git.submoduleForeach as jest.Mock<any, any>
mockSubmoduleForeach.mockClear() // reset calls

// Act
await authHelper.configureSubmoduleAuth()

// Assert
expect(mockSubmoduleForeach).toBeCalledTimes(1)
expect(mockSubmoduleForeach.mock.calls[0][0] as string).toMatch(
/unset-all.*insteadOf/
)
expect(mockSubmoduleForeach.mock.calls[2][0]).toMatch(/core\.sshCommand/)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

assert core.sshCommand was added

}
)

Expand Down
6 changes: 3 additions & 3 deletions action.yml
View file
Open in desktop
Expand Up @@ -16,20 +16,20 @@ inputs:
commands. The post-job step removes the PAT.


We recommend creating a service account with the least permissions necessary.
We recommend using a service account with the least permissions necessary.
Also when generating a new PAT, select the least scopes necessary.


[Learn more about creating and using encrypted secrets](https://help.github.com/en/actions/automating-your-workflow-with-github-actions/creating-and-using-encrypted-secrets)
default: ${{ github.token }}
ssh-key:
description: >
SSH key used to fetch the repository. SSH key is configured with the local
SSH key used to fetch the repository. The SSH key is configured with the local
git config, which enables your scripts to run authenticated git commands.
The post-job step removes the SSH key.


We recommend creating a service account with the least permissions necessary.
We recommend using a service account with the least permissions necessary.


[Learn more about creating and using
Expand Down
27 changes: 19 additions & 8 deletions dist/index.js
View file
Open in desktop
Expand Up @@ -5122,6 +5122,7 @@ class GitAuthHelper {
this.tokenConfigKey = `http.https://${HOSTNAME}/.extraheader`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

dont review index.js (generated)

this.insteadOfKey = `url.https://${HOSTNAME}/.insteadOf`;
this.insteadOfValue = `git@${HOSTNAME}:`;
this.sshCommand = '';
this.sshKeyPath = '';
this.sshKnownHostsPath = '';
this.temporaryHomePath = '';
Expand Down Expand Up @@ -5205,8 +5206,12 @@ class GitAuthHelper {
core.debug(`Replacing token placeholder in '${configPath}'`);
this.replaceTokenPlaceholder(configPath);
}
// Configure HTTPS instead of SSH
if (!this.settings.sshKey) {
if (this.settings.sshKey) {
// Configure core.sshCommand
yield this.git.submoduleForeach(`git config --local '${SSH_COMMAND_KEY}' '${this.sshCommand}'`, this.settings.nestedSubmodules);
}
else {
// Configure HTTPS instead of SSH
yield this.git.submoduleForeach(`git config --local '${this.insteadOfKey}' '${this.insteadOfValue}'`, this.settings.nestedSubmodules);
}
}
Expand Down Expand Up @@ -5268,16 +5273,16 @@ class GitAuthHelper {
yield fs.promises.writeFile(this.sshKnownHostsPath, knownHosts);
// Configure GIT_SSH_COMMAND
const sshPath = yield io.which('ssh', true);
let sshCommand = `"${sshPath}" -i "$RUNNER_TEMP/${path.basename(this.sshKeyPath)}"`;
this.sshCommand = `"${sshPath}" -i "$RUNNER_TEMP/${path.basename(this.sshKeyPath)}"`;
if (this.settings.sshStrict) {
sshCommand += ' -o StrictHostKeyChecking=yes -o CheckHostIP=no';
this.sshCommand += ' -o StrictHostKeyChecking=yes -o CheckHostIP=no';
}
sshCommand += ` -o "UserKnownHostsFile=$RUNNER_TEMP/${path.basename(this.sshKnownHostsPath)}"`;
core.info(`Temporarily overriding GIT_SSH_COMMAND=${sshCommand}`);
this.git.setEnvironmentVariable('GIT_SSH_COMMAND', sshCommand);
this.sshCommand += ` -o "UserKnownHostsFile=$RUNNER_TEMP/${path.basename(this.sshKnownHostsPath)}"`;
core.info(`Temporarily overriding GIT_SSH_COMMAND=${this.sshCommand}`);
this.git.setEnvironmentVariable('GIT_SSH_COMMAND', this.sshCommand);
// Configure core.sshCommand
if (this.settings.persistCredentials) {
yield this.git.config(SSH_COMMAND_KEY, sshCommand);
yield this.git.config(SSH_COMMAND_KEY, this.sshCommand);
}
});
}
Expand Down Expand Up @@ -5820,6 +5825,12 @@ function getSource(settings) {
// Downloading using REST API
core.info(`The repository will be downloaded using the GitHub REST API`);
core.info(`To create a local Git repository instead, add Git ${gitCommandManager.MinimumGitVersion} or higher to the PATH`);
if (settings.submodules) {
throw new Error(`Input 'submodules' not supported when falling back to download using the GitHub REST API. To create a local Git repository instead, add Git ${gitCommandManager.MinimumGitVersion} or higher to the PATH.`);
}
else if (settings.sshKey) {
throw new Error(`Input 'ssh-key' not supported when falling back to download using the GitHub REST API. To create a local Git repository instead, add Git ${gitCommandManager.MinimumGitVersion} or higher to the PATH.`);
}
yield githubApiHelper.downloadRepository(settings.authToken, settings.repositoryOwner, settings.repositoryName, settings.ref, settings.commit, settings.repositoryPath);
return;
}
Expand Down
23 changes: 15 additions & 8 deletions src/git-auth-helper.ts
View file
Open in desktop
Expand Up @@ -37,6 +37,7 @@ class GitAuthHelper {
private readonly tokenPlaceholderConfigValue: string
private readonly insteadOfKey: string = `url.https://${HOSTNAME}/.insteadOf`
private readonly insteadOfValue: string = `git@${HOSTNAME}:`
private sshCommand = ''
private sshKeyPath = ''
private sshKnownHostsPath = ''
private temporaryHomePath = ''
Expand Down Expand Up @@ -144,8 +145,14 @@ class GitAuthHelper {
this.replaceTokenPlaceholder(configPath)
}

// Configure HTTPS instead of SSH
if (!this.settings.sshKey) {
if (this.settings.sshKey) {
// Configure core.sshCommand
await this.git.submoduleForeach(
`git config --local '${SSH_COMMAND_KEY}' '${this.sshCommand}'`,
this.settings.nestedSubmodules
)
} else {
// Configure HTTPS instead of SSH
await this.git.submoduleForeach(
`git config --local '${this.insteadOfKey}' '${this.insteadOfValue}'`,
this.settings.nestedSubmodules
Expand Down Expand Up @@ -218,21 +225,21 @@ class GitAuthHelper {

// Configure GIT_SSH_COMMAND
const sshPath = await io.which('ssh', true)
let sshCommand = `"${sshPath}" -i "$RUNNER_TEMP/${path.basename(
this.sshCommand = `"${sshPath}" -i "$RUNNER_TEMP/${path.basename(
this.sshKeyPath
)}"`
if (this.settings.sshStrict) {
sshCommand += ' -o StrictHostKeyChecking=yes -o CheckHostIP=no'
this.sshCommand += ' -o StrictHostKeyChecking=yes -o CheckHostIP=no'
}
sshCommand += ` -o "UserKnownHostsFile=$RUNNER_TEMP/${path.basename(
this.sshCommand += ` -o "UserKnownHostsFile=$RUNNER_TEMP/${path.basename(
this.sshKnownHostsPath
)}"`
core.info(`Temporarily overriding GIT_SSH_COMMAND=${sshCommand}`)
this.git.setEnvironmentVariable('GIT_SSH_COMMAND', sshCommand)
core.info(`Temporarily overriding GIT_SSH_COMMAND=${this.sshCommand}`)
this.git.setEnvironmentVariable('GIT_SSH_COMMAND', this.sshCommand)

// Configure core.sshCommand
if (this.settings.persistCredentials) {
await this.git.config(SSH_COMMAND_KEY, sshCommand)
await this.git.config(SSH_COMMAND_KEY, this.sshCommand)
}
}

Expand Down
10 changes: 10 additions & 0 deletions src/git-source-provider.ts
View file
Open in desktop
Expand Up @@ -57,6 +57,16 @@ export async function getSource(settings: IGitSourceSettings): Promise<void> {
core.info(
`To create a local Git repository instead, add Git ${gitCommandManager.MinimumGitVersion} or higher to the PATH`
)
if (settings.submodules) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fail when fallback to REST API and submodules or ssh-key is set

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we do this before logging 'The repository will be downloaded using the GitHub REST API'

Since errors propagate up as annotations. Consider: "Git ${gitCommandManager.MinimumGitVersion} version or greater not found in path. Input 'submodules' not supported when falling back to download using the GitHub REST API"

Same for ssh key

Copy link
Contributor

@thboop thboop Mar 12, 2020 •
edited

Choose a reason for hiding this comment

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

I really like failing as an error flow here 👍 , as opposed to trying to download without submodules or without using the ssh key

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. To make the error actionable. I switched the order a little bit from the suggestion, to lead with the error message followed by suggested action.

throw new Error(
`Input 'submodules' not supported when falling back to download using the GitHub REST API. To create a local Git repository instead, add Git ${gitCommandManager.MinimumGitVersion} or higher to the PATH.`
)
} else if (settings.sshKey) {
throw new Error(
`Input 'ssh-key' not supported when falling back to download using the GitHub REST API. To create a local Git repository instead, add Git ${gitCommandManager.MinimumGitVersion} or higher to the PATH.`
)
}

await githubApiHelper.downloadRepository(
settings.authToken,
settings.repositoryOwner,
Expand Down