Skip to content

Set goprivate experiment for Go modules with git_source credentials #1526

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+216 −2
Open

Set goprivate experiment for Go modules with git_source credentials #1526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
185 changes: 185 additions & 0 deletions __tests__/updater.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,4 +401,189 @@ describe('Updater', () => {
])
})
})

describe('Go module configuration', () => {
describe('when package manager is go_modules and git_source credentials are present', () => {
const jobDetails = {
...mockJobDetails,
'package-manager': 'go_modules'
}

new Updater(
'MOCK_UPDATER_IMAGE_NAME',
'MOCK_PROXY_IMAGE_NAME',
mockApiClient,
jobDetails,
[
{
type: 'git_source',
host: 'github.com',
username: 'user',
password: 'pass'
},
{
type: 'npm_registry',
host: 'registry.npmjs.org',
username: 'npm_user',
token: 'npm_token'
}
],
workingDirectory
)

it('sets the goprivate experiment to *', () => {
expect(jobDetails.experiments).toEqual({
goprivate: '*'
})
})
})

describe('when package manager is go_modules but no git_source credentials are present', () => {
const jobDetails = {
...mockJobDetails,
'package-manager': 'go_modules'
}

new Updater(
'MOCK_UPDATER_IMAGE_NAME',
'MOCK_PROXY_IMAGE_NAME',
mockApiClient,
jobDetails,
[
{
type: 'npm_registry',
host: 'registry.npmjs.org',
username: 'npm_user',
token: 'npm_token'
}
],
workingDirectory
)

it('does not set the goprivate experiment', () => {
expect(jobDetails.experiments).toBeUndefined()
})
})

describe('when package manager is go_modules with existing experiments and git_source credentials are present', () => {
const jobDetails = {
...mockJobDetails,
'package-manager': 'go_modules',
experiments: {
'existing-experiment': 'value'
}
}

new Updater(
'MOCK_UPDATER_IMAGE_NAME',
'MOCK_PROXY_IMAGE_NAME',
mockApiClient,
jobDetails,
[
{
type: 'git_source',
host: 'private-git.com',
username: 'user',
password: 'pass'
}
],
workingDirectory
)

it('preserves existing experiments and adds goprivate', () => {
expect(jobDetails.experiments).toEqual({
'existing-experiment': 'value',
goprivate: '*'
})
})
})

describe('when package manager is not go_modules but git_source credentials are present', () => {
const jobDetails = {
...mockJobDetails,
'package-manager': 'npm-and-yarn'
}

new Updater(
'MOCK_UPDATER_IMAGE_NAME',
'MOCK_PROXY_IMAGE_NAME',
mockApiClient,
jobDetails,
[
{
type: 'git_source',
host: 'github.com',
username: 'user',
password: 'pass'
}
],
workingDirectory
)

it('does not set the goprivate experiment', () => {
expect(jobDetails.experiments).toBeUndefined()
})
})

describe('when package manager is go_modules and multiple git_source credentials are present', () => {
const jobDetails = {
...mockJobDetails,
'package-manager': 'go_modules'
}

new Updater(
'MOCK_UPDATER_IMAGE_NAME',
'MOCK_PROXY_IMAGE_NAME',
mockApiClient,
jobDetails,
[
{
type: 'git_source',
host: 'github.com',
username: 'user1',
password: 'pass1'
},
{
type: 'git_source',
host: 'gitlab.com',
username: 'user2',
password: 'pass2'
}
],
workingDirectory
)

it('sets the goprivate experiment to *', () => {
expect(jobDetails.experiments).toEqual({
goprivate: '*'
})
})
})

describe('when package manager is go_modules and jit_access credentials are present without git_source', () => {
const jobDetails = {
...mockJobDetails,
'package-manager': 'go_modules'
}

new Updater(
'MOCK_UPDATER_IMAGE_NAME',
'MOCK_PROXY_IMAGE_NAME',
mockApiClient,
jobDetails,
[
{
type: 'jit_access',
host: 'github.com',
token: 'token'
}
],
workingDirectory
)

it('does not set the goprivate experiment', () => {
expect(jobDetails.experiments).toBeUndefined()
})
})
})
})
13 changes: 13 additions & 0 deletions dist/main/index.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/main/index.js.map
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/api-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export type JobDetails = {
// Reuse Credential here since it shares many of the same fields,
// but the job details contains no secrets
'credentials-metadata': Credential[]
experiments: object
experiments: Record<string, any>
}

export type JobError = {
Expand Down
16 changes: 16 additions & 0 deletions src/updater.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export class Updater {
this.outputHostPath = path.join(workingDirectory, 'output')
this.repoHostPath = path.join(workingDirectory, 'repo')
this.details['credentials-metadata'] = this.generateCredentialsMetadata()
this.setGoModuleConfiguration()
}

/**
Expand Down Expand Up @@ -105,6 +106,21 @@ export class Updater {
return result
}

private setGoModuleConfiguration(): void {
Copy link
Member

Choose a reason for hiding this comment

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

This seems like it should go in Core in the Go ecosystem code. Any reason this can't be done in the same place as dependabot/dependabot-core#12747?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a valid question, I think it could go there. I put it here because it was previously in the job description, and this is the next place up where we have the full picture.

The way it is currently implemented in core it's a little tricky to centralize it where you pointed to, we pull out the value in a bunch of different classes and they get instantiated from the updater.

However, there are some more edge cases around how we handle this together with the go.env file, so I have a branch of core locally where I'm reshuffling the whole thing to just set these instance variables for the toolchain once, centralized in that one spot, and this actually is a perfect fit for that.

I can move it there, it does end up becoming a relatively large change, adding this will make it a bit bigger even, but I think it's manageable overall:

 12 files changed, 141 insertions(+), 87 deletions(-)

Copy link
Member

Choose a reason for hiding this comment

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

I think it's the right thing to do, having if package_manager == X spread around in different places isn't sustainable. Also consider we'll need to do this in the CLI so ADO can also use private Go Proxies.

This is a good opportunity to remove the goprivate code from the API as well. It seems all the info we need for this is in the credentials-metadata already, and have the goprivate settings as an "experiment" has always seemed odd.

// if any git sources are present for Go modules,
// set the goprivate experiment to '*', which will cause us to configure the
// Go toolchain with the `GOPRIVATE=*` environment variable.
if (this.details['package-manager'] === 'go_modules') {
const gitSources = this.credentials.filter(
credential => credential.type === 'git_source'
)
if (gitSources.length > 0) {
this.details.experiments = this.details.experiments || {}
this.details.experiments['goprivate'] = '*'
}
}
}

private setRegistryFromUrl(obj: Credential, credential: Credential): void {
const typesThatUseRegistryAsHost = [
'npm_registry',
Expand Down
Loading