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

feat(site launch): Allow for ops to remove timed out domain associations #720

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
2 changes: 1 addition & 1 deletion .env-example
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export SITE_CREATE_FORM_KEY=""
# generate your own access key and secret access key from AWS
export AWS_ACCESS_KEY_ID=""
export AWS_SECRET_ACCESS_KEY=""
export MOCK_AMPLIFY_CREATE_DOMAIN_CALLS="true"
export MOCK_AMPLIFY_DOMAIN_ASSOCIATION_CALLS="true"

# Required to run end-to-end tests
export E2E_TEST_REPO="e2e-test-repo"
Expand Down
6 changes: 3 additions & 3 deletions src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,9 @@ const config = convict({
format: String,
default: "",
},
mockAmplifyCreateDomainAssociationCalls: {
doc: "Mock createDomainAssociation calls to Amplify ",
env: "MOCK_AMPLIFY_CREATE_DOMAIN_ASSOCIATION_CALLS",
mockAmplifyDomainAssociationCalls: {
doc: "Mock domain association calls to Amplify",
env: "MOCK_AMPLIFY_DOMAIN_ASSOCIATION_CALLS",
format: "required-boolean",
default: true,
},
Expand Down
50 changes: 48 additions & 2 deletions src/services/identity/LaunchClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,16 @@ import {
GetDomainAssociationCommandInput as AmplifySDKGetDomainAssociationCommandInput,
GetDomainAssociationCommandOutput,
SubDomainSetting,
DeleteDomainAssociationCommandInput as AmplifySDKDeleteDomainAssociationCommandInput,
DeleteDomainAssociationCommand,
NotFoundException,
} from "@aws-sdk/client-amplify"
import { SubDomain } from "aws-sdk/clients/amplify"

import { config } from "@config/config"

import { AmplifyError } from "@root/types"

// stricter typing to interact with Amplify SDK
type CreateDomainAssociationCommandInput = {
[K in keyof AmplifySDKCreateDomainAssociationCommandInput]: NonNullable<
Expand All @@ -25,6 +30,19 @@ type GetDomainAssociationCommandInput = {
>
}

type DeleteDomainAssociationCommandInput = {
[K in keyof AmplifySDKDeleteDomainAssociationCommandInput]: NonNullable<
AmplifySDKDeleteDomainAssociationCommandInput[K]
>
}

export type AmplifyDomainNotFoundException = AmplifyError | NotFoundException

export const isAmplifyDomainNotFoundException = (
obj: unknown
): obj is AmplifyDomainNotFoundException =>
obj instanceof AmplifyError || obj instanceof NotFoundException

class LaunchClient {
private readonly amplifyClient: InstanceType<typeof AmplifyClient>

Expand Down Expand Up @@ -68,6 +86,14 @@ class LaunchClient {
domainName,
})

createDeleteDomainAssociationCommandInput = (
appId: string,
domainName: string
): DeleteDomainAssociationCommandInput => ({
appId,
domainName,
})

sendGetDomainAssociationCommand = (
input: GetDomainAssociationCommandInput
): Promise<GetDomainAssociationCommandOutput> => {
Expand Down Expand Up @@ -119,7 +145,7 @@ class LaunchClient {
}

private shouldMockAmplifyDomainCalls(): boolean {
return config.get("aws.amplify.mockAmplifyCreateDomainAssociationCalls")
return config.get("aws.amplify.mockAmplifyDomainAssociationCalls")
}

private getSubDomains(
Expand All @@ -142,13 +168,33 @@ class LaunchClient {
return subDomains
}

async sendDeleteDomainAssociationCommand(
input: DeleteDomainAssociationCommandInput
): Promise<void> {
if (this.shouldMockAmplifyDomainCalls()) {
this.mockDeleteDomainAssociationOutput(input)
}
await this.amplifyClient.send(new DeleteDomainAssociationCommand(input))
}

mockDeleteDomainAssociationOutput(
input: DeleteDomainAssociationCommandInput
) {
if (!this.mockDomainAssociations.has(input.domainName)) {
throw new AmplifyError(
`NotFoundException: Domain association ${input.domainName} not found.`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This mocks the behaviour of the real amplify client and throws an exception if domain hasnt been created yet

)
}
this.mockDomainAssociations.delete(input.domainName)
}

private mockGetDomainAssociationOutput(
input: GetDomainAssociationCommandInput
): Promise<GetDomainAssociationCommandOutput> {
const isSubDomainCreated = true // this is a `get` call, assume domain has already been created
const subDomainSettings = this.mockDomainAssociations.get(input.domainName)
if (!subDomainSettings) {
throw new Error(
throw new AmplifyError(
`NotFoundException: Domain association ${input.domainName} not found.`
)
}
Expand Down
39 changes: 38 additions & 1 deletion src/services/identity/LaunchesService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import logger from "@logger/logger"
import { Deployment, Launch, Repo, User, Redirection } from "@database/models"
import { RedirectionTypes } from "@root/constants/constants"
import { AmplifyError } from "@root/types/index"
import LaunchClient from "@services/identity/LaunchClient"
import LaunchClient, {
isAmplifyDomainNotFoundException,
} from "@services/identity/LaunchClient"

export type SiteLaunchCreateParams = {
userId: number
Expand Down Expand Up @@ -159,6 +161,41 @@ export class LaunchesService {
throw siteIdResult.error
}

try {
Copy link
Contributor Author

@kishore03109 kishore03109 Apr 20, 2023

Choose a reason for hiding this comment

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

Wrapped in a try catch since AmplifySDK will throw an (expected) not found error, which is still the happy path

// Check if association already exists
const getDomainAssociationCommandInput = this.launchClient.createGetDomainAssociationCommandInput(
appIdResult.value,
domainName
)

const getDomainAssociationResult = await this.launchClient.sendGetDomainAssociationCommand(
getDomainAssociationCommandInput
)
const hasDomainAssociationFailed =
getDomainAssociationResult?.domainAssociation?.domainStatus === "FAILED"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refer here for types of domain association result

if (hasDomainAssociationFailed) {
// safe to delete and retry
const deleteDomainAssociationCommandInput = this.launchClient.createDeleteDomainAssociationCommandInput(
appIdResult.value,
domainName
)
await this.launchClient.sendDeleteDomainAssociationCommand(
deleteDomainAssociationCommandInput
)
}
} catch (error: unknown) {
const isExpectedNotFoundError = isAmplifyDomainNotFoundException(error)
if (!isExpectedNotFoundError) {
return err(
new AmplifyError(
`Unable to connect to Amplify for: ${repoName}, ${error}`,
repoName,
appIdResult.value
)
)
}
}

const launchAppOptions = this.launchClient.createDomainAssociationCommandInput(
appIdResult.value,
domainName,
Expand Down
5 changes: 3 additions & 2 deletions src/services/infra/InfraService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,9 @@ export default class InfraService {
)

// Get DNS records from Amplify
const isTestEnv =
config.get("env") === "test" || config.get("env") === "dev"
const isTestEnv = config.get(
"aws.amplify.mockAmplifyDomainAssociationCalls"
)
// since we mock values during development, we don't have to await for the dns records
if (!isTestEnv) {
/**
Expand Down