Skip to content

add support for repository variables (#798) #819

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

Merged
merged 4 commits into from
May 28, 2025
Merged
Changes from all commits
Commits
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
196 changes: 196 additions & 0 deletions lib/plugins/variables.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
const _ = require('lodash')
const Diffable = require('./diffable')

module.exports = class Variables extends Diffable {
constructor (...args) {
super(...args)

if (this.entries) {
// Force all names to uppercase to avoid comparison issues.
this.entries.forEach((variable) => {
variable.name = variable.name.toUpperCase()
})
}
}

/**
* Look-up existing variables for a given repository
*
* @see {@link https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#list-repository-variables} list repository variables
* @returns {Array.<object>} Returns a list of variables that exist in a repository
*/
async find () {
this.log.debug(`Finding repo vars for ${this.repo.owner}/${this.repo.repo}`)
const { data: { variables } } = await this.github.request('GET /repos/:org/:repo/actions/variables', {
org: this.repo.owner,
repo: this.repo.repo
})
return variables
}

/**
* Compare the existing variables with what we've defined as code
*
* @param {Array.<object>} existing Existing variables defined in the repository
* @param {Array.<object>} variables Variables that we have defined as code
*
* @returns {object} The results of a list comparison
*/
getChanged (existing, variables = []) {
const result =
JSON.stringify(
existing.sort((x1, x2) => {
return x1.name.toUpperCase().localeCompare(x2.name.toUpperCase())
})
) !==
JSON.stringify(
variables.sort((x1, x2) => {
return x1.name.toUpperCase().localeCompare(x2.name.toUpperCase())
})
)
return result
Comment on lines +40 to +51
Copy link
Preview

Copilot AI May 28, 2025

Choose a reason for hiding this comment

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

Comparing two arrays via JSON.stringify is fragile. Consider using a deep comparison helper like _.isEqual on sorted arrays for more robust change detection.

Suggested change
const result =
JSON.stringify(
existing.sort((x1, x2) => {
return x1.name.toUpperCase().localeCompare(x2.name.toUpperCase())
})
) !==
JSON.stringify(
variables.sort((x1, x2) => {
return x1.name.toUpperCase().localeCompare(x2.name.toUpperCase())
})
)
return result
const sortedExisting = existing.sort((x1, x2) => {
return x1.name.toUpperCase().localeCompare(x2.name.toUpperCase());
});
const sortedVariables = variables.sort((x1, x2) => {
return x1.name.toUpperCase().localeCompare(x2.name.toUpperCase());
});
const result = !_.isEqual(sortedExisting, sortedVariables);
return result;

Copilot uses AI. Check for mistakes.

}

/**
* Compare existing variables with what's defined
*
* @param {Object} existing The existing entries in GitHub
* @param {Object} attrs The entries defined as code
*
* @returns
*/
comparator (existing, attrs) {
return existing.name === attrs.name
}

/**
* Return a list of changed entries
*
* @param {Object} existing The existing entries in GitHub
* @param {Object} attrs The entries defined as code
*
* @returns
*/
changed (existing, attrs) {
return this.getChanged(_.castArray(existing), _.castArray(attrs))
}

/**
* Update an existing variable if the value has changed
*
* @param {Array.<object>} existing Existing variables defined in the repository
* @param {Array.<object>} variables Variables that we have defined as code
*
* @see {@link https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#update-a-repository-variable} update a repository variable
* @returns
*/
async update (existing, variables = []) {
this.log.debug(`Updating a repo var existing params ${JSON.stringify(existing)} and new ${JSON.stringify(variables)}`)
existing = _.castArray(existing)
variables = _.castArray(variables)
const changed = this.getChanged(existing, variables)

if (changed) {
let existingVariables = [...existing]
for (const variable of variables) {
const existingVariable = existingVariables.find((_var) => _var.name === variable.name)
if (existingVariable) {
existingVariables = existingVariables.filter((_var) => _var.name !== variable.name)
if (existingVariable.value !== variable.value) {
await this.github
.request('PATCH /repos/:org/:repo/actions/variables/:variable_name', {
org: this.repo.owner,
repo: this.repo.repo,
variable_name: variable.name.toUpperCase(),
value: variable.value.toString()
})
.then((res) => {
return res
})
.catch((e) => {
this.logError(e)
})
}
} else {
await this.github
.request('POST /repos/:org/:repo/actions/variables', {
org: this.repo.owner,
repo: this.repo.repo,
name: variable.name.toUpperCase(),
value: variable.value.toString()
})
.then((res) => {
return res
})
.catch((e) => {
this.logError(e)
})
}
}

for (const variable of existingVariables) {
await this.github
.request('DELETE /repos/:org/:repo/actions/variables/:variable_name', {
org: this.repo.owner,
repo: this.repo.repo,
variable_name: variable.name.toUpperCase()
})
.then((res) => {
return res
})
.catch((e) => {
this.logError(e)
})
Comment on lines +100 to +143
Copy link
Preview

Copilot AI May 28, 2025

Choose a reason for hiding this comment

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

You’re using await and then chaining .then()/.catch(), which is redundant. Use try { await this.github.request(...) } catch (e) { this.logError(e) } for clearer async error handling.

Suggested change
await this.github
.request('PATCH /repos/:org/:repo/actions/variables/:variable_name', {
org: this.repo.owner,
repo: this.repo.repo,
variable_name: variable.name.toUpperCase(),
value: variable.value.toString()
})
.then((res) => {
return res
})
.catch((e) => {
this.logError(e)
})
}
} else {
await this.github
.request('POST /repos/:org/:repo/actions/variables', {
org: this.repo.owner,
repo: this.repo.repo,
name: variable.name.toUpperCase(),
value: variable.value.toString()
})
.then((res) => {
return res
})
.catch((e) => {
this.logError(e)
})
}
}
for (const variable of existingVariables) {
await this.github
.request('DELETE /repos/:org/:repo/actions/variables/:variable_name', {
org: this.repo.owner,
repo: this.repo.repo,
variable_name: variable.name.toUpperCase()
})
.then((res) => {
return res
})
.catch((e) => {
this.logError(e)
})
try {
await this.github.request('PATCH /repos/:org/:repo/actions/variables/:variable_name', {
org: this.repo.owner,
repo: this.repo.repo,
variable_name: variable.name.toUpperCase(),
value: variable.value.toString()
})
} catch (e) {
this.logError(e)
}
}
} else {
try {
await this.github.request('POST /repos/:org/:repo/actions/variables', {
org: this.repo.owner,
repo: this.repo.repo,
name: variable.name.toUpperCase(),
value: variable.value.toString()
})
} catch (e) {
this.logError(e)
}
}
}
for (const variable of existingVariables) {
try {
await this.github.request('DELETE /repos/:org/:repo/actions/variables/:variable_name', {
org: this.repo.owner,
repo: this.repo.repo,
variable_name: variable.name.toUpperCase()
})
} catch (e) {
this.logError(e)
}

Copilot uses AI. Check for mistakes.

}
}
}

/**
* Add a new variable to a given repository
*
* @param {object} variable The variable to add, with name and value
*
* @see {@link https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#create-a-repository-variable} create a repository variable
* @returns
*/
async add (variable) {
this.log.debug(`Adding a repo var with the params ${JSON.stringify(variable)}`)
Copy link
Preview

Copilot AI May 28, 2025

Choose a reason for hiding this comment

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

[nitpick] In add(), you rely on the constructor having uppercased variable.name. Consider explicitly normalizing variable.name = variable.name.toUpperCase() before sending the request to make the behavior clear.

Suggested change
this.log.debug(`Adding a repo var with the params ${JSON.stringify(variable)}`)
this.log.debug(`Adding a repo var with the params ${JSON.stringify(variable)}`)
variable.name = variable.name.toUpperCase()

Copilot uses AI. Check for mistakes.

await this.github
.request('POST /repos/:org/:repo/actions/variables', {
org: this.repo.owner,
repo: this.repo.repo,
name: variable.name,
value: variable.value.toString()
})
.then((res) => {
return res
})
.catch((e) => {
this.logError(e)
})
}

/**
* Remove variables that aren't defined as code
*
* @param {String} existing Name of the existing variable to remove
*
* @see {@link https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#delete-a-repository-variable} delete a repository variable
* @returns
*/
async remove (existing) {
this.log.debug(`Removing a repo var with the params ${JSON.stringify(existing)}`)
await this.github
.request('DELETE /repos/:org/:repo/actions/variables/:variable_name', {
org: this.repo.owner,
repo: this.repo.repo,
variable_name: existing.name
})
.then((res) => {
return res
})
.catch((e) => {
this.logError(e)
})
}
}
3 changes: 2 additions & 1 deletion lib/settings.js
Original file line number Diff line number Diff line change
@@ -960,7 +960,8 @@ Settings.PLUGINS = {
validator: require('./plugins/validator'),
rulesets: require('./plugins/rulesets'),
environments: require('./plugins/environments'),
custom_properties: require('./plugins/custom_properties.js')
custom_properties: require('./plugins/custom_properties.js'),
variables: require('./plugins/variables')
}

module.exports = Settings
79 changes: 79 additions & 0 deletions test/unit/lib/plugins/variables.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
const { when } = require('jest-when')
const Variables = require('../../../../lib/plugins/variables')

describe('Variables', () => {
let github
const org = 'bkeepers'
const repo = 'test'

function fillVariables (variables = []) {
return variables
}

function configure () {
const log = { debug: console.debug, error: console.error }
const errors = []
return new Variables(undefined, github, { owner: org, repo }, [{ name: 'test', value: 'test' }], log, errors)
}

beforeAll(() => {
github = {
request: jest.fn().mockReturnValue(Promise.resolve(true))
}
})

it('sync', () => {
const plugin = configure()

when(github.request)
.calledWith('GET /repos/:org/:repo/actions/variables', { org, repo })
.mockResolvedValue({
data: {
variables: [
fillVariables({
variables: []
})
Comment on lines +33 to +35
Copy link
Preview

Copilot AI May 28, 2025

Choose a reason for hiding this comment

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

fillVariables expects an array but is called with an object here; this mismatch can lead to mocks returning unexpected shapes. Pass an array of variable objects or adjust the helper to accept both shapes.

Suggested change
fillVariables({
variables: []
})
fillVariables([])

Copilot uses AI. Check for mistakes.

]
}
});

['variables'].forEach(() => {
when(github.request)
.calledWith('GET /repos/:org/:repo/actions/variables', { org, repo })
.mockResolvedValue({
data: {
variables: [{ name: 'DELETE_me', value: 'test' }]
}
})
})

when(github.request).calledWith('POST /repos/:org/:repo/actions/variables').mockResolvedValue({})

return plugin.sync().then(() => {
expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/actions/variables', { org, repo });

['variables'].forEach(() => {
expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/actions/variables', { org, repo })
})

expect(github.request).toHaveBeenCalledWith(
'DELETE /repos/:org/:repo/actions/variables/:variable_name',
expect.objectContaining({
org,
repo,
variable_name: 'DELETE_me'
})
)

expect(github.request).toHaveBeenCalledWith(
'POST /repos/:org/:repo/actions/variables',
expect.objectContaining({
org,
repo,
name: 'TEST',
value: 'test'
})
)
})
})
})