From 9d5c6633de70f166ab9b4b898d0ea072ed427e8a Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 28 Jan 2025 10:26:14 +0000 Subject: [PATCH] Generating short ID for prebuilds Also dropped unnecessary CASTs Signed-off-by: Danny Kopping --- coderd/database/queries.sql.go | 44 ++++++++++++--------------- coderd/database/queries/prebuilds.sql | 38 +++++++++++------------ coderd/prebuilds/controller.go | 25 ++++++++++++++- 3 files changed, 63 insertions(+), 44 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index e51357855a..0fb72e2610 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5422,7 +5422,7 @@ WITH INNER JOIN template_versions tv ON tv.template_id = t.id INNER JOIN template_version_presets tvp ON tvp.template_version_id = tv.id INNER JOIN template_version_preset_prebuilds tvpp ON tvpp.preset_id = tvp.id - WHERE t.id = $1::uuid + WHERE t.id = $1::uuid GROUP BY t.id, tv.id, tvpp.id), -- Jobs relating to prebuilds current in-flight prebuilds_in_progress AS (SELECT wpb.template_version_id, wpb.transition, COUNT(wpb.transition) AS count @@ -5435,29 +5435,25 @@ WITH GROUP BY wpb.template_version_id, wpb.transition) SELECT t.template_id, t.template_version_id, - t.using_active_version AS is_active, - CAST(COALESCE(MAX(CASE WHEN p.template_version_id = t.template_version_id THEN p.ids END), - '') AS TEXT) AS running_prebuild_ids, - CAST(COALESCE(MAX(CASE WHEN t.using_active_version THEN p.count ELSE 0 END), - 0) AS INT) AS actual, -- running prebuilds for active version - CAST(COALESCE(MAX(CASE WHEN t.using_active_version THEN t.desired_instances ELSE 0 END), - 0) AS INT) AS desired, -- we only care about the active version's desired instances - CAST(COALESCE(MAX(CASE WHEN p.template_version_id = t.template_version_id AND t.using_active_version = false THEN p.count END), - 0) AS INT) AS extraneous, -- running prebuilds for inactive version - CAST(COALESCE(MAX(CASE - WHEN pip.transition = 'start'::workspace_transition THEN pip.count - ELSE 0 END), - 0) AS INT) AS starting, - CAST(COALESCE(MAX(CASE - WHEN pip.transition = 'stop'::workspace_transition THEN pip.count - ELSE 0 END), - 0) AS INT) AS stopping, -- not strictly needed, since prebuilds should never be left if a "stopped" state, but useful to know - CAST(COALESCE(MAX(CASE - WHEN pip.transition = 'delete'::workspace_transition THEN pip.count - ELSE 0 END), - 0) AS INT) AS deleting, - t.deleted AS template_deleted, - t.deprecated AS template_deprecated + t.using_active_version AS is_active, + COALESCE(MAX(CASE WHEN p.template_version_id = t.template_version_id THEN p.ids END), + '')::text AS running_prebuild_ids, + COALESCE(MAX(CASE WHEN t.using_active_version THEN p.count ELSE 0 END), + 0)::int AS actual, -- running prebuilds for active version + COALESCE(MAX(CASE WHEN t.using_active_version THEN t.desired_instances ELSE 0 END), + 0)::int AS desired, -- we only care about the active version's desired instances + COALESCE(MAX(CASE + WHEN p.template_version_id = t.template_version_id AND t.using_active_version = false + THEN p.count END), + 0)::int AS extraneous, -- running prebuilds for inactive version + COALESCE(MAX(CASE WHEN pip.transition = 'start'::workspace_transition THEN pip.count ELSE 0 END), + 0)::int AS starting, + COALESCE(MAX(CASE WHEN pip.transition = 'stop'::workspace_transition THEN pip.count ELSE 0 END), + 0)::int AS stopping, -- not strictly needed, since prebuilds should never be left if a "stopped" state, but useful to know + COALESCE(MAX(CASE WHEN pip.transition = 'delete'::workspace_transition THEN pip.count ELSE 0 END), + 0)::int AS deleting, + t.deleted AS template_deleted, + t.deprecated AS template_deprecated FROM templates_with_prebuilds t LEFT JOIN running_prebuilds p ON p.template_version_id = t.template_version_id LEFT JOIN prebuilds_in_progress pip ON pip.template_version_id = t.template_version_id diff --git a/coderd/database/queries/prebuilds.sql b/coderd/database/queries/prebuilds.sql index 2291371fe2..28684bebae 100644 --- a/coderd/database/queries/prebuilds.sql +++ b/coderd/database/queries/prebuilds.sql @@ -38,25 +38,25 @@ WITH GROUP BY wpb.template_version_id, wpb.transition) SELECT t.template_id, t.template_version_id, - t.using_active_version AS is_active, - CAST(COALESCE(MAX(CASE WHEN p.template_version_id = t.template_version_id THEN p.ids END), - '') AS TEXT) AS running_prebuild_ids, - CAST(COALESCE(MAX(CASE WHEN t.using_active_version THEN p.count ELSE 0 END), - 0) AS INT) AS actual, -- running prebuilds for active version - CAST(COALESCE(MAX(CASE WHEN t.using_active_version THEN t.desired_instances ELSE 0 END), - 0) AS INT) AS desired, -- we only care about the active version's desired instances - CAST(COALESCE(MAX(CASE - WHEN p.template_version_id = t.template_version_id AND t.using_active_version = false - THEN p.count END), - 0) AS INT) AS extraneous, -- running prebuilds for inactive version - CAST(COALESCE(MAX(CASE WHEN pip.transition = 'start'::workspace_transition THEN pip.count ELSE 0 END), - 0) AS INT) AS starting, - CAST(COALESCE(MAX(CASE WHEN pip.transition = 'stop'::workspace_transition THEN pip.count ELSE 0 END), - 0) AS INT) AS stopping, -- not strictly needed, since prebuilds should never be left if a "stopped" state, but useful to know - CAST(COALESCE(MAX(CASE WHEN pip.transition = 'delete'::workspace_transition THEN pip.count ELSE 0 END), - 0) AS INT) AS deleting, - t.deleted AS template_deleted, - t.deprecated AS template_deprecated + t.using_active_version AS is_active, + COALESCE(MAX(CASE WHEN p.template_version_id = t.template_version_id THEN p.ids END), + '')::text AS running_prebuild_ids, + COALESCE(MAX(CASE WHEN t.using_active_version THEN p.count ELSE 0 END), + 0)::int AS actual, -- running prebuilds for active version + COALESCE(MAX(CASE WHEN t.using_active_version THEN t.desired_instances ELSE 0 END), + 0)::int AS desired, -- we only care about the active version's desired instances + COALESCE(MAX(CASE + WHEN p.template_version_id = t.template_version_id AND t.using_active_version = false + THEN p.count END), + 0)::int AS extraneous, -- running prebuilds for inactive version + COALESCE(MAX(CASE WHEN pip.transition = 'start'::workspace_transition THEN pip.count ELSE 0 END), + 0)::int AS starting, + COALESCE(MAX(CASE WHEN pip.transition = 'stop'::workspace_transition THEN pip.count ELSE 0 END), + 0)::int AS stopping, -- not strictly needed, since prebuilds should never be left if a "stopped" state, but useful to know + COALESCE(MAX(CASE WHEN pip.transition = 'delete'::workspace_transition THEN pip.count ELSE 0 END), + 0)::int AS deleting, + t.deleted AS template_deleted, + t.deprecated AS template_deprecated FROM templates_with_prebuilds t LEFT JOIN running_prebuilds p ON p.template_version_id = t.template_version_id LEFT JOIN prebuilds_in_progress pip ON pip.template_version_id = t.template_version_id diff --git a/coderd/prebuilds/controller.go b/coderd/prebuilds/controller.go index 003b0f1f99..03069cc7fd 100644 --- a/coderd/prebuilds/controller.go +++ b/coderd/prebuilds/controller.go @@ -2,6 +2,8 @@ package prebuilds import ( "context" + "crypto/rand" + "encoding/base32" "fmt" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database/dbtime" @@ -279,7 +281,10 @@ func (c Controller) reconcileTemplate(ctx context.Context, template database.Tem } func (c Controller) createPrebuild(ctx context.Context, db database.Store, prebuildID uuid.UUID, template database.Template) error { - name := fmt.Sprintf("prebuild-%s", prebuildID) + name, err := generateName() + if err != nil { + return xerrors.Errorf("failed to generate unique prebuild ID: %w", err) + } now := dbtime.Now() // Workspaces are created without any versions. @@ -355,3 +360,21 @@ func (c Controller) provision(ctx context.Context, db database.Store, prebuildID func (c Controller) Stop() { c.closeCh <- struct{}{} } + +// generateName generates a 20-byte prebuild name which should safe to use without truncation in most situations. +// UUIDs may be too long for a resource name in cloud providers (since this ID will be used in the prebuild's name). +// +// We're generating a 9-byte suffix (72 bits of entry): +// 1 - e^(-1e9^2 / (2 * 2^72)) = ~0.01% likelihood of collision in 1 billion IDs. +// See https://en.wikipedia.org/wiki/Birthday_attack. +func generateName() (string, error) { + b := make([]byte, 9) + + _, err := rand.Read(b) + if err != nil { + return "", err + } + + // Encode the bytes to Base32 (A-Z2-7), strip any '=' padding + return fmt.Sprintf("prebuild-%s", base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(b)), nil +}