Skip to content

Commit 4c356c7

Browse files
authored
Merge pull request #3570 from github/mbg/repo-props/warn-on-unexpected-props
Emit warning for unrecognised repo properties with our common prefix
2 parents dafe740 + b4937c1 commit 4c356c7

File tree

3 files changed

+71
-1
lines changed

3 files changed

+71
-1
lines changed

lib/init-action.js

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/feature-flags/properties.test.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import * as sinon from "sinon";
44
import * as api from "../api-client";
55
import { getRunnerLogger } from "../logging";
66
import { parseRepositoryNwo } from "../repository";
7-
import { setupTests } from "../testing-utils";
7+
import { RecordingLogger, setupTests } from "../testing-utils";
88

99
import * as properties from "./properties";
1010

@@ -197,3 +197,38 @@ test.serial(
197197
);
198198
},
199199
);
200+
201+
test.serial(
202+
"loadPropertiesFromApi warns if a repository property name starts with the common prefix, but is not recognised by us",
203+
async (t) => {
204+
process.env["GITHUB_EVENT_NAME"] = "push";
205+
const propertyName: string = `${properties.GITHUB_CODEQL_PROPERTY_PREFIX}unknown`;
206+
sinon.stub(api, "getRepositoryProperties").resolves({
207+
headers: {},
208+
status: 200,
209+
url: "",
210+
data: [
211+
{
212+
property_name: propertyName,
213+
value: "true",
214+
},
215+
] satisfies properties.GitHubPropertiesResponse,
216+
});
217+
const logger = new RecordingLogger();
218+
const warningSpy = sinon.spy(logger, "warning");
219+
const mockRepositoryNwo = parseRepositoryNwo("owner/repo");
220+
const response = await properties.loadPropertiesFromApi(
221+
logger,
222+
mockRepositoryNwo,
223+
);
224+
t.deepEqual(response, {});
225+
t.true(warningSpy.calledOnce);
226+
t.assert(
227+
warningSpy.firstCall.args[0]
228+
.toString()
229+
.startsWith(
230+
`Found repository properties ('${propertyName}'), which look like CodeQL Action repository properties`,
231+
),
232+
);
233+
},
234+
);

src/feature-flags/properties.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
1+
import { isDynamicWorkflow } from "../actions-util";
12
import { getRepositoryProperties } from "../api-client";
23
import { Logger } from "../logging";
34
import { RepositoryNwo } from "../repository";
45

6+
/** The common prefix that we expect all of our repository properties to have. */
7+
export const GITHUB_CODEQL_PROPERTY_PREFIX = "github-codeql-";
8+
59
/**
610
* Enumerates repository property names that have some meaning to us.
711
*/
@@ -114,6 +118,8 @@ export async function loadPropertiesFromApi(
114118
);
115119

116120
const properties: RepositoryProperties = {};
121+
const unrecognisedProperties: string[] = [];
122+
117123
for (const property of remoteProperties) {
118124
if (property.property_name === undefined) {
119125
throw new Error(
@@ -123,6 +129,11 @@ export async function loadPropertiesFromApi(
123129

124130
if (isKnownPropertyName(property.property_name)) {
125131
setProperty(properties, property.property_name, property.value, logger);
132+
} else if (
133+
property.property_name.startsWith(GITHUB_CODEQL_PROPERTY_PREFIX) &&
134+
!isDynamicWorkflow()
135+
) {
136+
unrecognisedProperties.push(property.property_name);
126137
}
127138
}
128139

@@ -139,6 +150,20 @@ export async function loadPropertiesFromApi(
139150
}
140151
}
141152

153+
// Emit a warning if we encountered unrecognised properties that have our prefix.
154+
if (unrecognisedProperties.length > 0) {
155+
const unrecognisedPropertyList = unrecognisedProperties
156+
.map((name) => `'${name}'`)
157+
.join(", ");
158+
159+
logger.warning(
160+
`Found repository properties (${unrecognisedPropertyList}), ` +
161+
"which look like CodeQL Action repository properties, " +
162+
"but which are not understood by this version of the CodeQL Action. " +
163+
"Do you need to update to a newer version?",
164+
);
165+
}
166+
142167
return properties;
143168
} catch (e) {
144169
throw new Error(

0 commit comments

Comments
 (0)