Skip to content

Commit

Permalink
fix(sync-mode): avoid collisions in sync key prefixes (#5409)
Browse files Browse the repository at this point in the history
* fix(sync-mode): avoid collisions in sync key prefixes

* refactor(sync-mode): change signature of `getSyncKeyPrefix`

* test(sync-mode): add unit tests

* chore: rename file
  • Loading branch information
vvagaytsev committed Nov 14, 2023
1 parent 71b3781 commit 9edc9ac
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 11 deletions.
49 changes: 39 additions & 10 deletions core/src/plugins/kubernetes/sync.ts
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
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
})
})
})

0 comments on commit 9edc9ac

Please sign in to comment.