Skip to content

Commit

Permalink
fix(glue-alpha): prefix validation logic is incorrect (#27472)
Browse files Browse the repository at this point in the history
There was logic in `aws-glue-alpha.Job` that expected `SparkUIProps.prefix` to be specified in the form `/prefix-name`, instead of the more conventional `prefix-name/`. This fix addresses the validation and handling of this prefix field to support the correct format. 


BREAKING CHANGE: `SparkUIProps.prefix` strings in the original `/prefix-name` format will now result in a validation error. To retain the same behavior, prefixes must be changed to the new `prefix-name/` format.

Closes #27396.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
mikewrighton authored Oct 10, 2023
1 parent 7d92a9c commit b898d3b
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 14 deletions.
18 changes: 10 additions & 8 deletions packages/@aws-cdk/aws-glue-alpha/lib/job.ts
Original file line number Diff line number Diff line change
Expand Up @@ -384,13 +384,13 @@ export interface SparkUIProps {
/**
* The bucket where the Glue job stores the logs.
*
* @default a new bucket will be created.
* @default - a new bucket will be created.
*/
readonly bucket?: s3.IBucket;

/**
* The path inside the bucket (objects prefix) where the Glue job stores the logs.
* Use format `'/foo/bar'`
* Use format `'foo/bar/'`
*
* @default - the logs will be written at the root of the bucket
*/
Expand All @@ -406,13 +406,15 @@ export interface SparkUIProps {
export interface SparkUILoggingLocation {
/**
* The bucket where the Glue job stores the logs.
*
* @default - a new bucket will be created.
*/
readonly bucket: s3.IBucket;

/**
* The path inside the bucket (objects prefix) where the Glue job stores the logs.
*
* @default '/' - the logs will be written at the root of the bucket
* @default - the logs will be written at the root of the bucket
*/
readonly prefix?: string;
}
Expand Down Expand Up @@ -840,12 +842,12 @@ export class Job extends JobBase {

const errors: string[] = [];

if (!prefix.startsWith('/')) {
errors.push('Prefix must begin with \'/\'');
if (prefix.startsWith('/')) {
errors.push('Prefix must not begin with \'/\'');
}

if (prefix.endsWith('/')) {
errors.push('Prefix must not end with \'/\'');
if (!prefix.endsWith('/')) {
errors.push('Prefix must end with \'/\'');
}

if (errors.length > 0) {
Expand All @@ -854,7 +856,7 @@ export class Job extends JobBase {
}

private cleanPrefixForGrant(prefix?: string): string | undefined {
return prefix !== undefined ? prefix.slice(1) + '/*' : undefined;
return prefix !== undefined ? `${prefix}*` : undefined;
}

private setupContinuousLogging(role: iam.IRole, props: ContinuousLoggingProps) {
Expand Down
12 changes: 6 additions & 6 deletions packages/@aws-cdk/aws-glue-alpha/test/job.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -632,14 +632,14 @@ describe('Job', () => {
});
describe('with bucket and path provided', () => {
const sparkUIBucketName = 'sparkbucketname';
const prefix = '/foob/bart';
const badPrefix = 'foob/bart/';
const prefix = 'foob/bart/';
const badPrefix = '/foob/bart';
let sparkUIBucket: s3.IBucket;

const expectedErrors = [
`Invalid prefix format (value: ${badPrefix})`,
'Prefix must begin with \'/\'',
'Prefix must not end with \'/\'',
'Prefix must not begin with \'/\'',
'Prefix must end with \'/\'',
].join(EOL);
it('fails if path is mis-formatted', () => {
expect(() => new glue.Job(stack, 'BadPrefixJob', {
Expand Down Expand Up @@ -699,7 +699,7 @@ describe('Job', () => {
[
'arn:',
{ Ref: 'AWS::Partition' },
`:s3:::sparkbucketname${prefix}/*`,
`:s3:::sparkbucketname/${prefix}*`,
],
],
},
Expand All @@ -718,7 +718,7 @@ describe('Job', () => {
Template.fromStack(stack).hasResourceProperties('AWS::Glue::Job', {
DefaultArguments: {
'--enable-spark-ui': 'true',
'--spark-event-logs-path': `s3://${sparkUIBucketName}${prefix}`,
'--spark-event-logs-path': `s3://${sparkUIBucketName}/${prefix}`,
},
});
});
Expand Down

0 comments on commit b898d3b

Please sign in to comment.