Skip to content

set insteadOf url for org-id#621

Merged
ericsciple merged 2 commits intomainfrom
users/ericsciple/21-10-org
Nov 1, 2021
Merged

set insteadOf url for org-id#621
ericsciple merged 2 commits intomainfrom
users/ericsciple/21-10-org

Conversation

@ericsciple
Copy link
Copy Markdown
Contributor

@ericsciple ericsciple commented Oct 22, 2021

fixes #570

When ssl certificate authentication is enabled, the clone UI suggested in the UI is like org-<ORG_ID>@github.com:<ORG>/<REPO>.git instead of the normal format [email protected]:<ORG>/<REPO>.git

We need to register a git config insteadOf url to handle the new format. Today users set the input submodules: true and the input ssh-key is not provided, we register an insteadOf config so SSH URLs beginning with [email protected]: are converted to HTTPS. We need to do the same thing for the new format.

@ericsciple ericsciple marked this pull request as ready for review November 1, 2021 15:27
Comment thread src/git-auth-helper.ts
`git config --local '${this.insteadOfKey}' '${this.insteadOfValue}'`,
this.settings.nestedSubmodules
)
for (const insteadOfValue of this.insteadOfValues) {
Copy link
Copy Markdown
Contributor Author

@ericsciple ericsciple Nov 1, 2021

Choose a reason for hiding this comment

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

this is for submodule config (recursive submodules)

Comment thread src/git-auth-helper.ts
this.insteadOfKey = `url.${serverUrl.origin}/.insteadOf` // "origin" is SCHEME://HOSTNAME[:PORT]
this.insteadOfValue = `git@${serverUrl.hostname}:`
this.insteadOfValues.push(`git@${serverUrl.hostname}:`)
if (this.settings.workflowOrganizationId) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

register extra prefix now

Comment thread src/git-auth-helper.ts
if (!this.settings.sshKey) {
await this.git.config(this.insteadOfKey, this.insteadOfValue, true)
for (const insteadOfValue of this.insteadOfValues) {
await this.git.config(this.insteadOfKey, insteadOfValue, true, true)
Copy link
Copy Markdown
Contributor Author

@ericsciple ericsciple Nov 1, 2021

Choose a reason for hiding this comment

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

when configuring auth, now we register multiple instead-of values

this translates submodule URLs like:

[email protected]:my-org/my-submodule.git -> https://github.com/my-org/my-submodule.git
[email protected]:my-org/my-submodule.git -> https://github.com/my-org/my-submodule.git

])
const args: string[] = ['config', globalConfig ? '--global' : '--local']
if (add) {
args.push('--add')
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is required in order to add two insteadOf URLs

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Try out running checkout on two separate occasions (i.e. the repo is not deleted in between runs).

/**
* Gets the organization ID of the running workflow or undefined if the value cannot be loaded from the GITHUB_EVENT_PATH
*/
export async function getOrganizationId(): Promise<number | undefined> {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

best effort, debug log if can't load

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

best effort, debug log if can't load

@juliobbv
Copy link
Copy Markdown

juliobbv commented Nov 1, 2021

Changes LGTM 👍 . The only thing pending to test is that --add scenario and we're good with merging.

@ericsciple ericsciple merged commit ec3a7ce into main Nov 1, 2021
@ericsciple ericsciple deleted the users/ericsciple/21-10-org branch November 1, 2021 16:43
Copy link
Copy Markdown

@Oriblish Oriblish left a comment

Choose a reason for hiding this comment

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

test/git-auth-helper.test.ts

/**
* Gets the organization ID of the running workflow or undefined if the value cannot be loaded from the GITHUB_EVENT_PATH
*/
export async function getOrganizationId(): Promise<number | undefined> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

best effort, debug log if can't load

Copy link
Copy Markdown

@Oriblish Oriblish left a comment

Choose a reason for hiding this comment

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

test/git-auth-helper.test.ts

@Oriblish
Copy link
Copy Markdown

#621 (review)

@Oriblish
Copy link
Copy Markdown

🧩

Copy link
Copy Markdown

@Oriblish Oriblish left a comment

Choose a reason for hiding this comment

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

test/git-auth-helper.test.ts``

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.

Organization ssh url will not support submodule checkout

3 participants