fix: address review feedback for actions CLI module
Fix argument parsing bugs in the execute command where actionId at index 0 was incorrectly skipped when --instance was absent, and flag values matching the actionId string were erroneously removed. Add --help support to the execute command for CLI report generation. Add missing bin script and cli-report.md for cli-module-actions. Add resolveAuth tests. Bump backend-defaults changeset to minor for the security-relevant auth change. Replace cross-fetch with native fetch in cli-module-auth to avoid punycode deprecation warnings during CLI report generation. Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com> Made-with: Cursor
This commit is contained in:
@@ -0,0 +1,32 @@
|
||||
#!/usr/bin/env node
|
||||
/*
|
||||
* Copyright 2025 The Backstage Authors
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
const path = require('node:path');
|
||||
|
||||
/* eslint-disable-next-line no-restricted-syntax */
|
||||
const isLocal = require('node:fs').existsSync(
|
||||
path.resolve(__dirname, '../src'),
|
||||
);
|
||||
|
||||
if (isLocal) {
|
||||
require('@backstage/cli-node/config/nodeTransform.cjs');
|
||||
}
|
||||
|
||||
const { runCliModule } = require('@backstage/cli-node');
|
||||
const cliModule = require(isLocal ? '../src/index' : '..').default;
|
||||
const pkg = require('../package.json');
|
||||
runCliModule({ module: cliModule, name: pkg.name, version: pkg.version });
|
||||
@@ -0,0 +1,94 @@
|
||||
## CLI Report file for "@backstage/cli-module-actions"
|
||||
|
||||
> Do not edit this file. It is a report generated by `yarn build:api-reports`
|
||||
|
||||
### `backstage-cli-module-actions`
|
||||
|
||||
```
|
||||
Usage: @backstage/cli-module-actions [options] [command]
|
||||
|
||||
Options:
|
||||
-V, --version
|
||||
-h, --help
|
||||
|
||||
Commands:
|
||||
actions [command]
|
||||
help [command]
|
||||
```
|
||||
|
||||
### `backstage-cli-module-actions actions`
|
||||
|
||||
```
|
||||
Usage: @backstage/cli-module-actions actions [options] [command] [command]
|
||||
|
||||
Options:
|
||||
-h, --help
|
||||
|
||||
Commands:
|
||||
execute
|
||||
help [command]
|
||||
list
|
||||
sources [command]
|
||||
```
|
||||
|
||||
### `backstage-cli-module-actions actions execute`
|
||||
|
||||
```
|
||||
Usage: @backstage/cli-module-actions actions execute
|
||||
|
||||
Options:
|
||||
--instance <string>
|
||||
-h, --help
|
||||
```
|
||||
|
||||
### `backstage-cli-module-actions actions list`
|
||||
|
||||
```
|
||||
Usage: @backstage/cli-module-actions actions list
|
||||
|
||||
Options:
|
||||
--instance <string>
|
||||
-h, --help
|
||||
```
|
||||
|
||||
### `backstage-cli-module-actions actions sources`
|
||||
|
||||
```
|
||||
Usage: @backstage/cli-module-actions actions sources [options] [command] [command]
|
||||
|
||||
Options:
|
||||
-h, --help
|
||||
|
||||
Commands:
|
||||
add
|
||||
help [command]
|
||||
list
|
||||
remove
|
||||
```
|
||||
|
||||
### `backstage-cli-module-actions actions sources add`
|
||||
|
||||
```
|
||||
Usage: @backstage/cli-module-actions actions sources add
|
||||
|
||||
Options:
|
||||
-h, --help
|
||||
```
|
||||
|
||||
### `backstage-cli-module-actions actions sources list`
|
||||
|
||||
```
|
||||
Usage: @backstage/cli-module-actions actions sources list
|
||||
|
||||
Options:
|
||||
-h, --help
|
||||
```
|
||||
|
||||
### `backstage-cli-module-actions actions sources remove`
|
||||
|
||||
```
|
||||
Usage: @backstage/cli-module-actions actions sources remove
|
||||
|
||||
Options:
|
||||
-h, --help
|
||||
```
|
||||
@@ -20,8 +20,10 @@
|
||||
"main": "src/index.ts",
|
||||
"types": "src/index.ts",
|
||||
"files": [
|
||||
"dist"
|
||||
"dist",
|
||||
"bin"
|
||||
],
|
||||
"bin": "bin/backstage-cli-module-actions",
|
||||
"scripts": {
|
||||
"build": "backstage-cli package build",
|
||||
"clean": "backstage-cli package clean",
|
||||
|
||||
@@ -21,12 +21,45 @@ import { schemaToFlags } from '../lib/schemaToFlags';
|
||||
import { resolveAuth } from '../lib/resolveAuth';
|
||||
|
||||
export default async ({ args, info }: CliCommandContext) => {
|
||||
if (args.includes('--help') || args.includes('-h')) {
|
||||
cli(
|
||||
{
|
||||
help: info,
|
||||
parameters: ['<action-id>'],
|
||||
flags: {
|
||||
instance: {
|
||||
type: String,
|
||||
description: 'Name of the instance to use',
|
||||
},
|
||||
},
|
||||
},
|
||||
undefined,
|
||||
args,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const instanceIdx = args.indexOf('--instance');
|
||||
const instanceFlag = instanceIdx !== -1 ? args[instanceIdx + 1] : undefined;
|
||||
|
||||
const actionId = args.find(
|
||||
(a, i) => !a.startsWith('-') && i !== instanceIdx + 1,
|
||||
);
|
||||
// Skip flag names, flag values (the argument after a known flag), and
|
||||
// the --instance value position so we only pick up positional arguments.
|
||||
const skipIndices = new Set<number>();
|
||||
if (instanceIdx !== -1) {
|
||||
skipIndices.add(instanceIdx);
|
||||
skipIndices.add(instanceIdx + 1);
|
||||
}
|
||||
|
||||
let actionId: string | undefined;
|
||||
let actionIdIdx = -1;
|
||||
for (let i = 0; i < args.length; i++) {
|
||||
if (!skipIndices.has(i) && !args[i].startsWith('-')) {
|
||||
actionId = args[i];
|
||||
actionIdIdx = i;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (!actionId) {
|
||||
process.stderr.write('Usage: actions execute <action-id> [flags]\n');
|
||||
process.exit(1);
|
||||
@@ -46,7 +79,7 @@ export default async ({ args, info }: CliCommandContext) => {
|
||||
|
||||
const schemaFlags = schemaToFlags(action.schema.input as any);
|
||||
|
||||
const flagArgs = args.filter(a => a !== actionId);
|
||||
const flagArgs = args.filter((_, i) => i !== actionIdIdx);
|
||||
|
||||
const { flags } = cli(
|
||||
{
|
||||
|
||||
@@ -0,0 +1,124 @@
|
||||
/*
|
||||
* Copyright 2025 The Backstage Authors
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
import { resolveAuth } from './resolveAuth';
|
||||
import {
|
||||
getSelectedInstance,
|
||||
getInstanceConfig,
|
||||
accessTokenNeedsRefresh,
|
||||
refreshAccessToken,
|
||||
getSecretStore,
|
||||
type StoredInstance,
|
||||
} from '@backstage/cli-module-auth';
|
||||
|
||||
jest.mock('@backstage/cli-module-auth', () => ({
|
||||
getSelectedInstance: jest.fn(),
|
||||
getInstanceConfig: jest.fn(),
|
||||
accessTokenNeedsRefresh: jest.fn(),
|
||||
refreshAccessToken: jest.fn(),
|
||||
getSecretStore: jest.fn(),
|
||||
}));
|
||||
|
||||
const mockGetSelectedInstance = getSelectedInstance as jest.MockedFunction<
|
||||
typeof getSelectedInstance
|
||||
>;
|
||||
const mockGetInstanceConfig = getInstanceConfig as jest.MockedFunction<
|
||||
typeof getInstanceConfig
|
||||
>;
|
||||
const mockAccessTokenNeedsRefresh =
|
||||
accessTokenNeedsRefresh as jest.MockedFunction<
|
||||
typeof accessTokenNeedsRefresh
|
||||
>;
|
||||
const mockRefreshAccessToken = refreshAccessToken as jest.MockedFunction<
|
||||
typeof refreshAccessToken
|
||||
>;
|
||||
const mockGetSecretStore = getSecretStore as jest.MockedFunction<
|
||||
typeof getSecretStore
|
||||
>;
|
||||
|
||||
describe('resolveAuth', () => {
|
||||
const mockInstance: StoredInstance = {
|
||||
name: 'production',
|
||||
baseUrl: 'https://backstage.example.com',
|
||||
clientId: 'my-client',
|
||||
issuedAt: Date.now(),
|
||||
accessTokenExpiresAt: Date.now() + 3600_000,
|
||||
};
|
||||
|
||||
const mockSecretStore = {
|
||||
get: jest.fn(),
|
||||
set: jest.fn(),
|
||||
delete: jest.fn(),
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
mockGetSelectedInstance.mockResolvedValue(mockInstance);
|
||||
mockAccessTokenNeedsRefresh.mockReturnValue(false);
|
||||
mockGetSecretStore.mockResolvedValue(mockSecretStore);
|
||||
mockSecretStore.get.mockResolvedValue('test-access-token');
|
||||
mockGetInstanceConfig.mockResolvedValue(['catalog', 'scaffolder']);
|
||||
});
|
||||
|
||||
it('resolves auth with the selected instance and stored token', async () => {
|
||||
const result = await resolveAuth();
|
||||
|
||||
expect(mockGetSelectedInstance).toHaveBeenCalledWith(undefined);
|
||||
expect(mockAccessTokenNeedsRefresh).toHaveBeenCalledWith(mockInstance);
|
||||
expect(mockRefreshAccessToken).not.toHaveBeenCalled();
|
||||
expect(result).toEqual({
|
||||
instance: mockInstance,
|
||||
accessToken: 'test-access-token',
|
||||
pluginSources: ['catalog', 'scaffolder'],
|
||||
});
|
||||
});
|
||||
|
||||
it('passes instance name flag to getSelectedInstance', async () => {
|
||||
await resolveAuth('staging');
|
||||
|
||||
expect(mockGetSelectedInstance).toHaveBeenCalledWith('staging');
|
||||
});
|
||||
|
||||
it('refreshes the access token when it is about to expire', async () => {
|
||||
const refreshedInstance = {
|
||||
...mockInstance,
|
||||
accessTokenExpiresAt: Date.now() + 7200_000,
|
||||
};
|
||||
mockAccessTokenNeedsRefresh.mockReturnValue(true);
|
||||
mockRefreshAccessToken.mockResolvedValue(refreshedInstance);
|
||||
|
||||
const result = await resolveAuth();
|
||||
|
||||
expect(mockRefreshAccessToken).toHaveBeenCalledWith('production');
|
||||
expect(result.instance).toBe(refreshedInstance);
|
||||
});
|
||||
|
||||
it('throws when no access token is stored', async () => {
|
||||
mockSecretStore.get.mockResolvedValue(undefined);
|
||||
|
||||
await expect(resolveAuth()).rejects.toThrow(
|
||||
'No access token found. Run "auth login" to authenticate.',
|
||||
);
|
||||
});
|
||||
|
||||
it('returns empty plugin sources when none are configured', async () => {
|
||||
mockGetInstanceConfig.mockResolvedValue(undefined);
|
||||
|
||||
const result = await resolveAuth();
|
||||
|
||||
expect(result.pluginSources).toEqual([]);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user