modified service and service tests to work with the correct error format

clopca · Jun 28, 2024 · ba122ac · ba122ac
1 parent e7d2563
commit ba122ac
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 30 deletions.
diff --git a/src/service.ts b/src/service.ts
@@ -171,25 +171,39 @@ export class Service extends ServiceBase {
   // Imports
   // ------------------------------------------------------
   public static fromServiceArn(scope: Construct, id: string, serviceArn: string): IService {
-    validateServiceArn();
-
     class Import extends ServiceBase {
       public readonly serviceArn = serviceArn;
-      public readonly serviceId = core.Arn.extractResourceName(serviceArn, 'service');
-    }
-    return new Import(scope, id);
+      public readonly serviceId: string;
+
+      constructor() {
+        super(scope, id);
+        this.serviceId = this.extractServiceId();
+        this.node.addValidation({ validate: () => this.validateServiceArn() });
+      }
 
-    function validateServiceArn() {
-      const arnPattern = /^arn:aws:vpc-lattice:[a-z0-9-]+:\d{12}:service\/[a-zA-Z0-9-]+$/;
+      private extractServiceId(): string {
+        try {
+          return core.Arn.extractResourceName(this.serviceArn, 'service');
+        } catch (error) {
+          // If extraction fails, return an empty string or some placeholder
+          // The validation will catch this and report the error
+          return '';
+        }
+      }
 
-      if (!arnPattern.test(serviceArn)) {
-        throw new Error(`Service ARN should be in the format 'arn:aws:vpc-lattice:<REGION>:<ACCOUNT>:service/<NAME>', got ${serviceArn}.`);
+      private validateServiceArn(): string[] {
+        const errors: string[] = [];
+        const arnPattern = /^arn:aws:vpc-lattice:[a-z0-9-]+:\d{12}:service\/[a-zA-Z0-9-]+$/;
+        if (!arnPattern.test(this.serviceArn)) {
+          errors.push(`Service ARN should be in the format 'arn:aws:vpc-lattice:<REGION>:<ACCOUNT>:service/<NAME>', got ${this.serviceArn}.`);
+        }
+        return errors;
       }
     }
+    return new Import();
   }
   // -----------
   public static fromServiceId(scope: Construct, id: string, serviceId: string): IService {
-    validateServiceId();
     class Import extends ServiceBase {
       public readonly serviceId = serviceId;
       public readonly serviceArn = core.Arn.format(
@@ -198,20 +212,26 @@ export class Service extends ServiceBase {
           resource: 'service',
           resourceName: serviceId,
         },
-        core.Stack.of(this),
+        core.Stack.of(scope),
       );
-    }
-    return new Import(scope, id);
-
-    function validateServiceId() {
-      // Combined pattern to check the "svc-" prefix and apply the name pattern
-      const idPattern = /^svc-(?!svc-)(?!-)(?!.*-$)(?!.*--)[a-z0-9-]{3,40}$/;
-      if (!idPattern.test(serviceId)) {
-        throw new Error(
-          `Service ID should be in the format 'svc-<NAME>', where <NAME> is 3-40 characters long, starts and ends with a letter or number, cannot start with "svc-", and can contain lowercase letters, numbers, and hyphens (no consecutive hyphens). Got ${serviceId}.`,
-        );
+
+      constructor() {
+        super(scope, id);
+        this.node.addValidation({ validate: () => this.validateServiceId() });
+      }
+
+      private validateServiceId(): string[] {
+        const errors: string[] = [];
+        const idPattern = /^svc-(?!svc-)(?!-)(?!.*-$)(?!.*--)[a-z0-9-]{3,40}$/;
+        if (!idPattern.test(this.serviceId)) {
+          errors.push(
+            `Service ID should be in the format 'svc-<NAME>', where <NAME> is 3-40 characters long, starts and ends with a letter or number, cannot start with "svc-", and can contain lowercase letters, numbers, and hyphens (no consecutive hyphens). Got ${this.serviceId}.`,
+          );
+        }
+        return errors;
       }
     }
+    return new Import();
   }
 
   // -----------

diff --git a/test/core/unit/service.test.ts b/test/core/unit/service.test.ts
@@ -579,7 +579,8 @@ describe('Service', () => {
   describe('Import', () => {
     test('Import fromServiceArn', () => {
       // GIVEN
-      const stack = new cdk.Stack();
+      const app = new cdk.App();
+      const stack = new cdk.Stack(app, 'TestStack');
 
       // WHEN
       const service = Service.fromServiceArn(stack, 'ImportedService', 'arn:aws:vpc-lattice:us-west-2:123456789012:service/svc-12345abcdef');
@@ -591,7 +592,8 @@ describe('Service', () => {
 
     test('Import fromServiceArn', () => {
       // GIVEN
-      const stack = new cdk.Stack();
+      const app = new cdk.App();
+      const stack = new cdk.Stack(app, 'TestStack');
       const arn = 'arn:aws:vpc-lattice:us-west-2:123456789012:service/svc-12345abcdef';
 
       // WHEN
@@ -604,7 +606,8 @@ describe('Service', () => {
 
     test('Import with wrong arn format', () => {
       // GIVEN
-      const stack = new cdk.Stack();
+      const app = new cdk.App();
+      const stack = new cdk.Stack(app, 'TestStack');
       const invalidArns = [
         'arn:aws:ec2:us-west-2:123456789012:service/svc-12345abcdef', // Wrong service
         'arn:aws:vpc-lattice:us-west-2:123456789012:vpc/svc-12345abcdef', // Wrong resource type
@@ -618,15 +621,15 @@ describe('Service', () => {
 
       // WHEN & THEN
       invalidArns.forEach(invalidArn => {
-        expect(() => {
-          Service.fromServiceArn(stack, 'ImportedService', invalidArn);
-        }).toThrow(/Service ARN should be in the format/);
+        Service.fromServiceArn(stack, `ImportedService-${invalidArn}`, invalidArn);
+        expect(() => app.synth()).toThrow(/Service ARN should be in the format/);
       });
     });
 
     test('Import fromServiceId', () => {
       // GIVEN
-      const stack = new cdk.Stack();
+      const app = new cdk.App();
+      const stack = new cdk.Stack(app, 'TestStack');
 
       // WHEN
       const service = Service.fromServiceId(stack, 'ImportedService', 'svc-12345abcdef');
@@ -640,12 +643,14 @@ describe('Service', () => {
 
     test('Import with wrong serviceId', () => {
       // GIVEN
-      const stack = new cdk.Stack();
+      const app = new cdk.App();
+      const stack = new cdk.Stack(app, 'TestStack');
       const invalidIds = ['12345abcdef', 'svc-12345ABcd', 'svc--abc123', 'svc-svc-12345abc/abc', 'svc-abc--ab', 'svc-a', `svc-${'x'.repeat(41)}`];
 
       // WHEN & THEN
       invalidIds.forEach(invalidId => {
-        expect(() => Service.fromServiceId(stack, 'ImportedService', invalidId)).toThrow(/Service ID should be in the format/);
+        Service.fromServiceId(stack, `ImportedService-${invalidId}`, invalidId);
+        expect(() => app.synth()).toThrow(/Service ID should be in the format/);
       });
     });
   });