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

fix(sync-mode): avoid collisions in sync key prefixes #5409

Merged
merged 4 commits into from
Nov 14, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 39 additions & 10 deletions core/src/plugins/kubernetes/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,11 @@ export async function startSyncs(params: StartSyncsParams) {
)

const allSyncs = expectedKeys.length === 0 ? [] : await mutagen.getActiveSyncSessions()
const keyPrefix = getSyncKeyPrefix(ctx, action)
const keyPrefix = getSyncKeyPrefix({
environmentName: ctx.environmentName,
namespace: ctx.namespace,
actionName: action.name,
})

for (const sync of allSyncs.filter((s) => s.name.startsWith(keyPrefix) && !expectedKeys.includes(s.name))) {
log.info(`Terminating unexpected/outdated sync ${sync.name}`)
Expand All @@ -615,7 +619,11 @@ export async function stopSyncs(params: StopSyncsParams) {
const mutagen = new Mutagen({ ctx, log })

const allSyncs = await mutagen.getActiveSyncSessions()
const keyPrefix = getSyncKeyPrefix(ctx, action)
const keyPrefix = getSyncKeyPrefix({
environmentName: ctx.environmentName,
namespace: ctx.namespace,
actionName: action.name,
})
const syncs = allSyncs.filter((sync) => sync.name.startsWith(keyPrefix))

for (const sync of syncs) {
Expand Down Expand Up @@ -775,7 +783,11 @@ export async function getSyncStatus(params: GetSyncStatusParams): Promise<GetSyn
})
}

const keyPrefix = getSyncKeyPrefix(ctx, action)
const keyPrefix = getSyncKeyPrefix({
environmentName: ctx.environmentName,
namespace: ctx.namespace,
actionName: action.name,
})

let extraSyncs = false

Expand Down Expand Up @@ -807,21 +819,38 @@ export async function getSyncStatus(params: GetSyncStatusParams): Promise<GetSyn
}
}

function getSyncKeyPrefix(ctx: PluginContext, action: SupportedRuntimeAction) {
return kebabCase(`k8s--${ctx.environmentName}--${ctx.namespace}--${action.name}--`)
interface StructuredSyncKeyPrefix {
environmentName: string
namespace: string
actionName: string
}

export function getSyncKeyPrefix({ environmentName, namespace, actionName }: StructuredSyncKeyPrefix) {
// Kebab-case each part of the key prefix separately to preserve double-dash delimiters
return `k8s--${kebabCase(environmentName)}--${kebabCase(namespace)}--${kebabCase(actionName)}--`
}

/**
* Generates a unique key for sa single sync.
* IMPORTANT!!! The key will be used as an argument in the `mutagen` shell command.
* It cannot contain any characters that can break the command execution (like / \ < > | :).
* It cannot contain any characters that can break the command execution (like / \ < > | :).
*
* Note, that function {@link kebabCase} replaces any sequence of multiple dashes with a single dash character.
*
* It's critical to have double dashes (--) as a delimiter of a key parts here and in {@link getSyncKeyPrefix}
* to avoid potential collisions of the sync key prefixes.
*/
function getSyncKey({ ctx, action, spec }: PrepareSyncParams, target: SyncableResource): string {
export function getSyncKey({ ctx, action, spec }: PrepareSyncParams, target: SyncableResource): string {
const sourcePath = relative(action.sourcePath(), spec.sourcePath)
const containerPath = spec.containerPath
return kebabCase(
`${getSyncKeyPrefix(ctx, action)}${target.kind}--${target.metadata.name}--${sourcePath}--${containerPath}`
)
// Kebab-case each part of the key prefix separately to preserve double-dash delimiters
return `${getSyncKeyPrefix({
environmentName: ctx.environmentName,
namespace: ctx.namespace,
actionName: action.name,
})}${kebabCase(target.kind)}--${kebabCase(target.metadata.name)}--${kebabCase(sourcePath)}--${kebabCase(
containerPath
)}`
}

async function prepareSync(params: PrepareSyncParams) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@
*/

import { expect } from "chai"
import { builtInExcludes, getLocalSyncPath, makeSyncConfig } from "../../../../../src/plugins/kubernetes/sync.js"
import {
builtInExcludes,
getLocalSyncPath,
getSyncKeyPrefix,
makeSyncConfig,
} from "../../../../../src/plugins/kubernetes/sync.js"

describe("k8s sync helpers", () => {
describe("getLocalSyncPath", () => {
Expand Down Expand Up @@ -141,4 +146,25 @@ describe("k8s sync helpers", () => {
})
})
})

describe("getSyncKeyPrefix", () => {
const environmentName = "dev"
const namespace = "default"

it("produces a sync key prefix with double-dashes", () => {
const syncKeyPrefix = getSyncKeyPrefix({ environmentName, namespace, actionName: "backend" })
expect(syncKeyPrefix).to.eql("k8s--dev--default--backend--")
})

it("produces non-colliding keys if one action's name starts with another action's name", () => {
const actionName1 = "backend"
const actionName2 = "backend-new"
expect(actionName2.startsWith(actionName1)).to.be.true

const syncKeyPrefix1 = getSyncKeyPrefix({ environmentName, namespace, actionName: actionName1 })
const syncKeyPrefix2 = getSyncKeyPrefix({ environmentName, namespace, actionName: actionName2 })
expect(syncKeyPrefix2.startsWith(syncKeyPrefix1)).to.be.false
expect(syncKeyPrefix1.startsWith(syncKeyPrefix2)).to.be.false
})
})
})