Skip to content
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

RDX: Work with deployment profiles #4655

Merged
merged 6 commits into from
May 18, 2023
Merged
Show file tree
Hide file tree
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
6 changes: 5 additions & 1 deletion background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1097,7 +1097,9 @@ class BackgroundCommandWorker implements CommandWorkerInterface {
if (state === 'install') {
console.debug(`Installing extension ${ image }...`);
try {
if (await extension.install()) {
const { enabled, list } = cfg.application.extensions.allowed;

if (await extension.install(enabled ? list : undefined)) {
return { status: 201 };
} else {
return { status: 204 };
Expand All @@ -1109,6 +1111,8 @@ class BackgroundCommandWorker implements CommandWorkerInterface {
return { status: 422, data: `The image ${ image } has invalid extension metadata` };
case ExtensionErrorCode.FILE_NOT_FOUND:
return { status: 422, data: `The image ${ image } failed to install: ${ ex.message }` };
case ExtensionErrorCode.INSTALL_DENIED:
return { status: 403, data: `The image ${ image } is not an allowed extension` };
}
}
throw ex;
Expand Down
131 changes: 131 additions & 0 deletions bats/tests/extensions/allow-list.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
load '../helpers/load'

setup() {
TESTDATA_DIR="${PATH_BATS_ROOT}/tests/extensions/testdata/"

if using_windows_exe; then
TESTDATA_DIR_CLI="$(wslpath -m "${TESTDATA_DIR}")"
else
TESTDATA_DIR_CLI="${TESTDATA_DIR}"
fi

if using_containerd; then
namespace_arg=('--namespace=rancher-desktop-extensions')
else
namespace_arg=()
fi
}

write_allow_list() { # list
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a test that passes more than one arg to this function, because currently it only handles 0 or 1 values in the list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It takes a single argument that is the JSON encoded list.

Copy link
Contributor

Choose a reason for hiding this comment

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

I realized that, but none of the args contain a comma

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I thought I had one. Will fix.

local list=${1-}
local allowed=true

if [ -z "$list" ]; then
allowed=false
fi

# Note that the list preference is not writable using `rdctl set`, and we
# need to do a direct API call instead.

rdctl api /v1/settings --input - <<<'{
"version": '"$(get_setting .version)"',
"application": {
"extensions": {
"allowed": {
"enabled": '"${allowed}"',
"list": '"${list:-[]}"'
}
}
}
}'
}

check_extension_installed() { # refute, name
run rdctl extension ls
assert_success
"${1:-assert}_output" --partial "${2:-rd/extension/basic}"
}

@test 'factory reset' {
factory_reset
}

@test 'start container engine' {
RD_ENV_EXTENSIONS=1 start_container_engine
wait_for_container_engine
}

@test 'build extension testing image' {
ctrctl "${namespace_arg[@]}" build \
--tag "rd/extension/basic" \
--build-arg "variant=basic" \
"$TESTDATA_DIR_CLI"

run ctrctl "${namespace_arg[@]}" image list --format '{{ .Repository }}'
assert_success
assert_line "rd/extension/basic"
}

@test 'when no extension allow list is set up, all extensions can install' {
write_allow_list ''
rdctl extension install rd/extension/basic
check_extension_installed
rdctl extension uninstall rd/extension/basic
}

@test 'empty allow list disables extension installs' {
write_allow_list '[]'
run rdctl extension install rd/extension/basic
assert_failure
check_extension_installed refute
}

@test 'when an extension is explicitly allowed, it can be installed' {
write_allow_list '["irrelevant/image","rd/extension/basic:latest"]'
rdctl extension install rd/extension/basic:latest
Copy link
Contributor

Choose a reason for hiding this comment

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

I would also run rdctl extension ls and check for success and basic:latest when the extension whitelist is in effect

check_extension_installed
rdctl extension uninstall rd/extension/basic
check_extension_installed refute
}

@test 'when an extension is not in the allowed list, it cannot be installed' {
write_allow_list '["rd/extension/other","registry.test/image"]'
run rdctl extension install rd/extension/basic
assert_failure
check_extension_installed refute
}

@test 'when no tags given, any tag is allowed' {
write_allow_list '["rd/extension/basic"]'
ctrctl "${namespace_arg[@]}" tag rd/extension/basic rd/extension/basic:0.0.3
rdctl extension install rd/extension/basic:0.0.3
check_extension_installed
rdctl extension uninstall rd/extension/basic
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, I would verify that the extension is actually installed, not just that ext install didn't fail

check_extension_installed refute
}

@test 'when tags are given, only the specified tag is allowed' {
sleep 20
write_allow_list '["rd/extension/basic:0.0.2"]'
ctrctl "${namespace_arg[@]}" tag rd/extension/basic rd/extension/basic:0.0.3
run rdctl extension install rd/extension/basic:0.0.3
assert_failure
check_extension_installed refute
}

@test 'extensions can be allowed by organization' {
write_allow_list '["rd/extension/"]'
rdctl extension install rd/extension/basic
check_extension_installed
rdctl extension uninstall rd/extension/basic
check_extension_installed refute
}

@test 'extensions can be allowed by repository host' {
write_allow_list '["registry.test/"]'
ctrctl "${namespace_arg[@]}" tag rd/extension/basic registry.test/basic:0.0.3
rdctl extension install registry.test/basic:0.0.3
check_extension_installed '' registry.test/basic
rdctl extension uninstall registry.test/basic
check_extension_installed refute registry.test/basic
}
14 changes: 14 additions & 0 deletions pkg/rancher-desktop/assets/specs/command-api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,20 @@ components:
debug:
type: boolean
x-rd-usage: generate more verbose logging
extensions:
type: object
properties:
allowed:
type: object
properties:
enabled:
type: boolean
x-rd-hidden: true
list:
type: array
items:
type: string
x-rd-hidden: true
pathManagementStrategy:
type: string
enum: [manual, rcfiles]
Expand Down
10 changes: 8 additions & 2 deletions pkg/rancher-desktop/config/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,14 @@ export enum CacheMode {
export const defaultSettings = {
version: CURRENT_SETTINGS_VERSION,
application: {
adminAccess: true,
debug: false,
adminAccess: true,
debug: false,
extensions: {
allowed: {
enabled: false,
list: [] as Array<string>,
},
},
pathManagementStrategy: PathManagementStrategy.NotSet,
telemetry: { enabled: true },
/** Whether we should check for updates and apply them. */
Expand Down
39 changes: 36 additions & 3 deletions pkg/rancher-desktop/main/commandServer/settingsValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
} from '@pkg/config/settings';
import { NavItemName, navItemNames, TransientSettings } from '@pkg/config/transientSettings';
import { PathManagementStrategy } from '@pkg/integrations/pathManager';
import { validateImageName, validateImageTag } from '@pkg/utils/dockerUtils';
import { parseImageReference, validateImageName, validateImageTag } from '@pkg/utils/dockerUtils';
import { RecursivePartial } from '@pkg/utils/typeUtils';
import { preferencesNavItems } from '@pkg/window/preferences';

Expand Down Expand Up @@ -73,8 +73,14 @@ export default class SettingsValidator {
this.allowedSettings ||= {
version: this.checkUnchanged,
application: {
adminAccess: this.checkLima(this.checkBoolean),
debug: this.checkBoolean,
adminAccess: this.checkLima(this.checkBoolean),
debug: this.checkBoolean,
extensions: {
allowed: {
enabled: this.checkBoolean,
list: this.checkExtensionAllowList,
},
},
pathManagementStrategy: this.checkLima(this.checkPathManagementStrategy),
telemetry: { enabled: this.checkBoolean },
/** Whether we should check for updates and apply them. */
Expand Down Expand Up @@ -548,6 +554,33 @@ export default class SettingsValidator {
return !_.isEqual(desiredValue, currentValue);
}

protected checkExtensionAllowList(
mergedSettings: Settings,
currentValue: string[],
desiredValue: any,
errors: string[],
fqname: string,
): boolean {
if (_.isEqual(desiredValue, currentValue)) {
// Accept no-op changes
return false;
}

const changed = this.checkUniqueStringArray(mergedSettings, currentValue, desiredValue, errors, fqname);

if (errors.length) {
return changed;
}

for (const pattern of desiredValue as string[]) {
if (!parseImageReference(pattern, true)) {
errors.push(`${ fqname }: "${ pattern }" does not describe an image reference`);
}
}

return errors.length === 0 && changed;
}

protected checkPreferencesNavItemCurrent(
mergedSettings: TransientSettings,
currentValue: NavItemName,
Expand Down
49 changes: 49 additions & 0 deletions pkg/rancher-desktop/main/extensions/__tests__/extensions.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { ExtensionImpl } from '@pkg/main/extensions/extensions';

describe('ExtensionImpl', () => {
describe('checkInstallAllowed', () => {
const subject = ExtensionImpl['checkInstallAllowed'];

it('should reject invalid image references', () => {
expect(() => subject(undefined, '/')).toThrow();
});

it('should allow images if the allow list is not enabled', () => {
expect(() => subject(undefined, 'image')).not.toThrow();
});

it('should disallow any images given an empty list', () => {
expect(() => subject([], 'image')).toThrow();
});

it('should allow specified image', () => {
expect(() => subject(['other', 'image'], 'image')).not.toThrow();
});

it('should reject unknown image', () => {
expect(() => subject(['allowed'], 'image')).toThrow();
});

it('should support missing tags', () => {
expect(() => subject(['image'], 'image:1.0.0')).not.toThrow();
});

it('should reject images with the wrong tag', () => {
expect(() => subject(['image:0.1'], 'image:0.2')).toThrow();
});

it('should support image references with registries', () => {
const ref = 'r.example.test:1234/org/name:tag';

expect(() => subject([ref], ref)).not.toThrow();
});

it('should support org-level references', () => {
expect(() => subject(['test.invalid/org/'], 'test.invalid/org/image:tag')).not.toThrow();
});

it('should support registry-level references', () => {
expect(() => subject(['registry.test/'], 'registry.test/image:tag')).not.toThrow();
});
});
});
59 changes: 55 additions & 4 deletions pkg/rancher-desktop/main/extensions/extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {

import type { ContainerEngineClient } from '@pkg/backend/containerClient';
import mainEvents from '@pkg/main/mainEvents';
import { parseImageReference } from '@pkg/utils/dockerUtils';
import Logging from '@pkg/utils/logging';
import paths from '@pkg/utils/paths';
import { defined } from '@pkg/utils/typeUtils';
Expand All @@ -24,9 +25,9 @@ class ExtensionErrorImpl extends Error implements ExtensionError {

constructor(code: ExtensionErrorCode, message: string, cause?: Error) {
// XXX We're currently using a version of TypeScript that doesn't have the
// ES2022.Error lib, so it does't understand the "cause" option to the Error
// constructor. Work around this by explicitly calling setting the cause.
// It appears to still be printed in that case.
// ES2022.Error lib, so it doesn't understand the "cause" option to the
// Error constructor. Work around this by explicitly calling setting the
// cause. It appears to still be printed in that case.
super(message);
if (cause) {
(this as any).cause = cause;
Expand Down Expand Up @@ -142,9 +143,59 @@ export class ExtensionImpl implements Extension {
return this._iconName as Promise<string>;
}

async install(): Promise<boolean> {
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Uncaught typo from an earlier PR: does't at line 28 of pkg/rancher-desktop/main/extensions/extensions.ts

* Check if the given image is allowed to be installed according to the
* extension allow list.
* @throws If the image is not allowed to be installed.
*/
protected static checkInstallAllowed(allowedImages: readonly string[] | undefined, image: string) {
const desired = parseImageReference(image);
const code = ExtensionErrorCode.INSTALL_DENIED;
const prefix = `Disallowing install of ${ image }:`;

if (!desired) {
throw new ExtensionErrorImpl(code, `${ prefix } Invalid image reference`);
}
if (!allowedImages) {
return;
}
for (const pattern of allowedImages) {
const allowed = parseImageReference(pattern, true);

if (allowed?.tag && allowed.tag !== desired.tag) {
// This pattern doesn't match the tag, look for something else.
continue;
}

if (allowed?.registry.href !== desired.registry.href) {
// This pattern has a different registry
continue;
}

if (!allowed.name) {
// If there's no name given, the whole registry is allowed.
return '';
}

if (allowed.name.endsWith('/')) {
if (desired.name.startsWith(allowed.name)) {
// The allowed pattern ends with a slash, anything in the org is fine.
return '';
}
} else if (allowed.name === desired.name) {
return '';
}
}

throw new ExtensionErrorImpl(code, `${ prefix } Image is not allowed`);
}

async install(allowedImages: readonly string[] | undefined): Promise<boolean> {
const metadata = await this.metadata;

ExtensionImpl.checkInstallAllowed(allowedImages, this.image);
console.debug(`Image ${ this.image } is allowed to install: ${ allowedImages }`);

await fs.promises.mkdir(this.dir, { recursive: true });
try {
await this.installMetadata(this.dir, metadata);
Expand Down
Loading