INT-7797: refactor private ca #587
Conversation
| const instance = | ||
| getRawData<privateca_v1.Schema$CertificateAuthority>(caAuthorityEntity); |
There was a problem hiding this comment.
nit:
| const instance = | |
| getRawData<privateca_v1.Schema$CertificateAuthority>(caAuthorityEntity); | |
| const caAuthority = | |
| getRawData<privateca_v1.Schema$CertificateAuthority>(caAuthorityEntity); |
There was a problem hiding this comment.
It typically makes sense to put this step inside fetchAuthorityCertificates.ts, since the relationship is built off of that entity.
| import { PrivatecaSteps } from '../constants'; | ||
|
|
||
| describe(`privateca#${PrivatecaSteps.STEP_PRIVATE_CA_CERTIFICATES.id}`, () => { | ||
| let recording: Recording; |
There was a problem hiding this comment.
tip: this suggestion will prevent you from ever forgetting the condition in front of if (recording) await recording.stop()
| let recording: Recording; | |
| let recording: Recording | undefined; |
| ); | ||
|
|
||
| if (!caPoolEntity || !caAuthorityEntity) { | ||
| return; |
There was a problem hiding this comment.
Should we maybe log something here?
| const bucketName = instance.gcsBucket; | ||
|
|
||
| if (!bucketName) { | ||
| return; |
| getCloudStorageBucketKey(bucketName), | ||
| ); | ||
| if (!storageBucketEntity) { | ||
| return; |
| logger.warn( | ||
| `${PrivatecaSteps.STEP_PRIVATE_CA_POOLS.id} - Found undefined caPool entity in iterateCaPools.`, | ||
| ); |
There was a problem hiding this comment.
Is this something you've encountered? How is the API returning an array that contains undefined?
There was a problem hiding this comment.
this was just for safe purposes but you are right it does not make much sense
| if (!bucketName) { | ||
| logger.warn( | ||
| `${PrivatecaSteps.STEP_CREATE_PRIVATE_CA_CERTIFICATE_AUTHORITY_BUCKET_RELATIONSHIPS.id} - Missing bucketName.`, | ||
| ); | ||
| return; | ||
| } |
There was a problem hiding this comment.
curious - is gcsBucket a required property here? Is it true that if we do not find it, something has gone wrong?
There was a problem hiding this comment.
| if (!bucketName) { | |
| logger.warn( | |
| `${PrivatecaSteps.STEP_CREATE_PRIVATE_CA_CERTIFICATE_AUTHORITY_BUCKET_RELATIONSHIPS.id} - Missing bucketName.`, | |
| ); | |
| return; | |
| } | |
| if (!bucketName) { | |
| return; | |
| } |
|
|
||
| const result = await executeStepWithDependencies(stepTestConfig); | ||
| expect(result).toMatchStepMetadata(stepTestConfig); | ||
| }); |
There was a problem hiding this comment.
Tip: instead of separately setting jest.setTimeout(45_000), you can add the timeout as the third positional argument to test():
| }); | |
| }, 45_000); |
gastonyelmini
added
minor
|
🚀 PR was released in |
j1-internal-automation
added
the
released
The issue is that we’re using privateca API version v1beta1, which is deprecated. We need to instead use v1, which is supported.
as a result, we need to also add the caPool resource, which should be a parent of google_certificate_authority.
This also includes: