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

Update cdk ecr images when docker hash changes #1080

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
14 changes: 1 addition & 13 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -200,20 +200,8 @@ jobs:
node-version: ${{ env.node-version }}
- name: Install NPM dependencies
run: npm ci
- name: Mock Docker Images for CDK Tests
run: |
image=$(docker images --format '{{.Repository}}:{{.Tag}}' | head -n 1)
echo "Using image for mocked tags: $image"
docker tag $image migrations/capture_proxy:latest
docker tag $image migrations/capture_proxy_es:latest
docker tag $image opensearchproject/opensearch:2
docker tag $image migrations/elasticsearch_searchguard:latest
docker tag $image docker.io/apache/kafka:3.7.0
docker tag $image migrations/migration_console:latest
docker tag $image migrations/reindex_from_snapshot:latest
docker tag $image migrations/traffic_replayer:latest
Comment on lines -203 to -214
Copy link
Collaborator

Choose a reason for hiding this comment

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

thanks for removing this

- name: Run CDK Jest Tests (using mocked images)
run: npm run test:withoutBuildImages
run: npm run test

link-checker:
runs-on: ubuntu-latest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ module.exports = {
transform: {
'^.+\\.tsx?$': 'ts-jest'
},
setupFilesAfterEnv: ['<rootDir>/test/jest.setup.ts'],
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ import { IStringParameter, StringParameter } from "aws-cdk-lib/aws-ssm";
import * as forge from 'node-forge';
import { ClusterYaml } from "./migration-services-yaml";
import { CdkLogger } from "./cdk-logger";

import { mkdtempSync, writeFileSync } from 'fs';
import { join } from 'path';
import { tmpdir } from 'os';
import { DockerImageAsset } from "aws-cdk-lib/aws-ecr-assets";
import { execSync } from 'child_process';
export function getSecretAccessPolicy(secretArn: string): PolicyStatement {
return new PolicyStatement({
effect: Effect.ALLOW,
Expand All @@ -16,10 +20,6 @@ export function getSecretAccessPolicy(secretArn: string): PolicyStatement {
]
})
}
import { mkdtempSync, writeFileSync } from 'fs';
import { join } from 'path';
import { tmpdir } from 'os';
import { DockerImageAsset } from "aws-cdk-lib/aws-ecr-assets";

export function appendArgIfNotInExtraArgs(
baseCommand: string,
Expand Down Expand Up @@ -453,17 +453,41 @@ export function isRegionGovCloud(region: string): boolean {
*
* @param {string} imageName - The name of the Docker image to save as a tarball and use in CDK.
* @returns {ContainerImage} - A `ContainerImage` object representing the Docker image asset.
*/
*/
export function makeLocalAssetContainerImage(scope: Construct, imageName: string): ContainerImage {
const sanitizedImageName = imageName.replace(/[^a-zA-Z0-9-_]/g, '_');
const tempDir = mkdtempSync(join(tmpdir(), 'docker-build-' + sanitizedImageName));
const dockerfilePath = join(tempDir, 'Dockerfile');
const dockerfileContent = `
FROM ${imageName}
`;
writeFileSync(dockerfilePath, dockerfileContent);
const asset = new DockerImageAsset(scope, 'ServiceImage', {
directory: tempDir,
});
return ContainerImage.fromDockerImageAsset(asset);
}
const sanitizedImageName = imageName.replace(/[^a-zA-Z0-9-_]/g, '_');
const tempDir = mkdtempSync(join(tmpdir(), 'docker-build-' + sanitizedImageName));
const dockerfilePath = join(tempDir, 'Dockerfile');

let imageHash = null;
try {
// Update the image if it is not a local image
if (!imageName.startsWith('migrations/')) {
execSync(`docker pull ${imageName}`);
}
// Get the actual hash for the image
const imageId = execSync(`docker inspect --format='{{.Id}}' ${imageName}`).toString().trim();
if (!imageId) {
throw new Error(`No RepoDigests found for image: ${imageName}`);
}
imageHash = imageId.split(':')[1];
CdkLogger.info('For image: ' + imageName + ' found hash: ' + imageHash);
} catch (error) {
CdkLogger.error('Error fetching the actual hash for the image: ' + imageName + ' Error: ' + error);
throw new Error('Error fetching the image hash for the image: ' + imageName + ' Error: ' + error);
}

const dockerfileContent = `
FROM ${imageName}
`;

writeFileSync(dockerfilePath, dockerfileContent);
const assetName = `${sanitizedImageName.charAt(0).toUpperCase() + sanitizedImageName.slice(1)}ImageAsset`;
const asset = new DockerImageAsset(scope, assetName, {
directory: tempDir,
// add the tag to the hash so that the asset is invalidated when the tag changes
extraHash: imageHash,
assetName: assetName,
});
return ContainerImage.fromDockerImageAsset(asset);
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
TaskDefinition
} from "aws-cdk-lib/aws-ecs";
import {LogGroup, RetentionDays} from "aws-cdk-lib/aws-logs";
import {createAwsDistroForOtelPushInstrumentationPolicy} from "../common-utilities";
import {createAwsDistroForOtelPushInstrumentationPolicy, makeLocalAssetContainerImage} from "../common-utilities";
import {Duration, RemovalPolicy} from "aws-cdk-lib";
import { DockerImageAsset } from "aws-cdk-lib/aws-ecr-assets";
import { join } from "path";
Expand Down Expand Up @@ -41,9 +41,7 @@ export class OtelCollectorSidecar {
logGroupName: `${logGroupPrefix}/otel-collector`
});
const otelCollectorContainer = taskDefinition.addContainer("OtelCollectorContainer", {
image: ContainerImage.fromDockerImageAsset(new DockerImageAsset(taskDefinition.stack, "OtelCollectorImage", {
directory: join(__dirname, "../../../../../", "TrafficCapture/dockerSolution/src/main/docker/otelCollector")
})),
image: makeLocalAssetContainerImage(taskDefinition.stack, "migrations/otel_collector:latest"),
containerName: "otel-collector",
command: ["--config=/etc/otel-config-aws.yaml"],
portMappings: [otelCollectorPort, otelCollectorHealthcheckPort],
Expand Down
3 changes: 1 addition & 2 deletions deployment/cdk/opensearch-service-migration/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@
"scripts": {
"build": "tsc",
"watch": "tsc -w",
"test": "./buildDockerImages.sh && npm run test:withoutBuildImages",
"test:withoutBuildImages": "jest --coverage",
"test": "jest --coverage",
"clean": "rm -rf dist",
"release": "npm run build",
"cdk": "cdk",
Expand Down
34 changes: 34 additions & 0 deletions deployment/cdk/opensearch-service-migration/test/jest.setup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@

jest.mock('child_process', () => {
const child_process = jest.requireActual('child_process');

// Define the list of expected Docker images as per CI.yml
const expectedDockerImages = [
'docker.io/apache/kafka:3.7.0',
'migrations/capture_proxy_es:latest',
'migrations/capture_proxy:latest',
'migrations/elasticsearch_searchguard:latest',
'migrations/migration_console:latest',
'migrations/reindex_from_snapshot:latest',
'migrations/traffic_replayer:latest',
'migrations/otel_collector:latest',
'opensearchproject/opensearch:2',
];

return {
execSync: jest.fn().mockImplementation((command: string) => {
if (expectedDockerImages.some(expectedImage => command.endsWith(expectedImage))) {
if (command.startsWith('docker inspect')) {
return Buffer.from("sha256:a6cdbbcf16c510f8e24c6b993");
}
if (command.startsWith('docker pull')) {
return Buffer.from("");
}
}
throw new Error(`Command not supported: ${command}`);
}),
// Uncomment and replace above to use the actual execSync, requires ./buildDockerImages.sh to be run locally
// execSync: child_process.execSync,
spawnSync: child_process.spawnSync,
};
});
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ describe('ACM Certificate Importer Handler', () => {
});

afterEach(() => {
jest.resetAllMocks();
jest.clearAllMocks();
jest.resetModules();
jest.restoreAllMocks();
});

test('Create: should generate and import a self-signed certificate', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ import {ContainerImage} from "aws-cdk-lib/aws-ecs";
import {describe, beforeEach, afterEach, test, expect, jest} from '@jest/globals';
import {ReindexFromSnapshotStack} from "../lib";


describe('Migration Console Stack Tests', () => {
// Mock the implementation of fromDockerImageAsset before all tests
beforeEach(() => {
// Mock the implementation of fromDockerImageAsset before all tests
jest.spyOn(ContainerImage, 'fromDockerImageAsset').mockImplementation(() => ContainerImage.fromRegistry("ServiceImage"));
});

Expand All @@ -16,7 +17,6 @@ describe('Migration Console Stack Tests', () => {
jest.clearAllMocks();
jest.resetModules();
jest.restoreAllMocks();
jest.resetAllMocks();
});

test('Migration Console task definition is updated when services.yaml inputs change', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ describe('Migration Services YAML Tests', () => {
jest.clearAllMocks();
jest.resetModules();
jest.restoreAllMocks();
jest.resetAllMocks();
});

test('Test default servicesYaml can be stringified', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ describe('NetworkStack Tests', () => {
jest.clearAllMocks();
jest.resetModules();
jest.restoreAllMocks();
jest.resetAllMocks();
});

test('Test vpcEnabled setting that is disabled does not create stack', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ describe('OpenSearch Domain Stack Tests', () => {
jest.clearAllMocks();
jest.resetModules();
jest.restoreAllMocks();
jest.resetAllMocks();
});

test('Test primary context options are mapped with standard data type', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ describe('ReindexFromSnapshotStack Tests', () => {
jest.clearAllMocks();
jest.resetModules();
jest.restoreAllMocks();
jest.resetAllMocks();
});

test('ReindexFromSnapshotStack creates expected resources', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ describe('Stack Composer Ordering Tests', () => {
jest.clearAllMocks();
jest.resetModules();
jest.restoreAllMocks();
jest.resetAllMocks();
});

test('Test all migration services with MSK get created when enabled', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ describe('Stack Composer Tests', () => {
jest.clearAllMocks();
jest.resetModules();
jest.restoreAllMocks();
jest.resetAllMocks();
});

test('Test empty string provided for a parameter which has a default value, uses the default value', () => {
Expand Down