Skip to content

Commit

Permalink
fix: JSII error while configuring image builders on Python (#469)
Browse files Browse the repository at this point in the history
JSII returned `Construct` instead of `RunnerImageBuilder` because `RunnerImageBuilder` inherits from `RunnerImageBuilderBase` which is marked internal. To resolve this, we instead return the same object as a new public interface called `IConfigurableImageBuilder`.

We still want vanilla `IRunnerImageBuilder` as someone might want their own version of a builder that may not need components or network configuration. Just like `StaticRunnerImage.fromEcrRepository()``.

This probably failed on Java, .NET and Go too.

Fixes #468
  • Loading branch information
kichik authored Nov 16, 2023
1 parent 64e5e61 commit 4c4e254
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 23 deletions.
98 changes: 92 additions & 6 deletions API.md

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

6 changes: 3 additions & 3 deletions src/image-builders/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import { Annotations } from 'aws-cdk-lib';
import { Construct } from 'constructs';
import { AwsImageBuilderRunnerImageBuilder } from './aws-image-builder';
import { CodeBuildRunnerImageBuilder } from './codebuild';
import { RunnerImageBuilderBase, RunnerImageBuilderProps, RunnerImageBuilderType } from './common';
import { Os } from '../providers/common';
import { IConfigurableRunnerImageBuilder, RunnerImageBuilderBase, RunnerImageBuilderProps, RunnerImageBuilderType } from './common';
import { Os } from '../providers';

/**
* GitHub Runner image builder. Builds a Docker image or AMI with GitHub Runner and other requirements installed.
Expand All @@ -16,7 +16,7 @@ export abstract class RunnerImageBuilder extends RunnerImageBuilderBase {
/**
* Create a new image builder based on the provided properties. The implementation will differ based on the OS, architecture, and requested builder type.
*/
static new(scope: Construct, id: string, props?: RunnerImageBuilderProps): RunnerImageBuilder {
static new(scope: Construct, id: string, props?: RunnerImageBuilderProps): IConfigurableRunnerImageBuilder {
if (props?.components && props.runnerVersion) {
Annotations.of(scope).addWarning('runnerVersion is ignored when components are specified. The runner version will be determined by the components.');
}
Expand Down
35 changes: 23 additions & 12 deletions src/image-builders/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Construct } from 'constructs';
import { AwsImageBuilderRunnerImageBuilderProps } from './aws-image-builder';
import { CodeBuildRunnerImageBuilderProps } from './codebuild';
import { RunnerImageComponent } from './components';
import { Architecture, Os, RunnerAmi, RunnerImage, RunnerVersion } from '../providers/common';
import { Architecture, Os, RunnerAmi, RunnerImage, RunnerVersion } from '../providers';

/**
* @internal
Expand Down Expand Up @@ -278,10 +278,31 @@ export interface IRunnerImageBuilder {
bindAmi(): RunnerAmi;
}

/**
* Interface for constructs that build an image that can be used in {@link IRunnerProvider}. The image can be configured by adding or removing components. The image builder can be configured by adding grants or allowing connections.
*
* An image can be a Docker image or AMI.
*/
export interface IConfigurableRunnerImageBuilder extends IRunnerImageBuilder, ec2.IConnectable, iam.IGrantable {
/**
* Add a component to the image builder. The component will be added to the end of the list of components.
*
* @param component component to add
*/
addComponent(component: RunnerImageComponent): void;

/**
* Remove a component from the image builder. Removal is done by component name. Multiple components with the same name will all be removed.
*
* @param component component to remove
*/
removeComponent(component: RunnerImageComponent): void;
}

/**
* @internal
*/
export abstract class RunnerImageBuilderBase extends Construct implements ec2.IConnectable, iam.IGrantable, IRunnerImageBuilder {
export abstract class RunnerImageBuilderBase extends Construct implements IConfigurableRunnerImageBuilder {
protected components: RunnerImageComponent[] = [];

protected constructor(scope: Construct, id: string, props?: RunnerImageBuilderProps) {
Expand All @@ -299,20 +320,10 @@ export abstract class RunnerImageBuilderBase extends Construct implements ec2.IC
abstract get connections(): ec2.Connections;
abstract get grantPrincipal(): iam.IPrincipal;

/**
* Add a component to the image builder. The component will be added to the end of the list of components.
*
* @param component component to add
*/
public addComponent(component: RunnerImageComponent) {
this.components.push(component);
}

/**
* Remove a component from the image builder. Removal is done by component name. Multiple components with the same name will all be removed.
*
* @param component component to remove
*/
public removeComponent(component: RunnerImageComponent) {
this.components = this.components.filter(c => c.name !== component.name);
}
Expand Down
2 changes: 1 addition & 1 deletion src/providers/ecs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ export class EcsRunnerProvider extends BaseProvider implements IRunnerProvider {
* * `RunnerImageComponent.docker()`
* * `RunnerImageComponent.githubRunner()`
*/
public static imageBuilder(scope: Construct, id: string, props?: RunnerImageBuilderProps): RunnerImageBuilder {
public static imageBuilder(scope: Construct, id: string, props?: RunnerImageBuilderProps) {
return RunnerImageBuilder.new(scope, id, {
os: Os.LINUX_UBUNTU,
architecture: Architecture.X86_64,
Expand Down
2 changes: 1 addition & 1 deletion src/providers/fargate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ export class FargateRunnerProvider extends BaseProvider implements IRunnerProvid
* * `RunnerImageComponent.awsCli()`
* * `RunnerImageComponent.githubRunner()`
*/
public static imageBuilder(scope: Construct, id: string, props?: RunnerImageBuilderProps): RunnerImageBuilder {
public static imageBuilder(scope: Construct, id: string, props?: RunnerImageBuilderProps) {
return RunnerImageBuilder.new(scope, id, {
os: Os.LINUX_UBUNTU,
architecture: Architecture.X86_64,
Expand Down

0 comments on commit 4c4e254

Please sign in to comment.