convert postgres_config steps to go by stevenolen · Pull Request #164 · posit-dev/ptd

stevenolen · 2026-03-05T19:53:10Z

Description

Refactors the aws control room & workload and azure workload postgres_config steps from python to go. This has been tested with a control room and 2 workloads, producing no diffs.

Category of change

Bug fix (non-breaking change which fixes an issue)
Version upgrade (upgrading the version of a service or product)
New feature (non-breaking change which adds functionality)
Build: a code change that affects the build system or external dependencies
Performance: a code change that improves performance
Refactor: a code change that neither fixes a bug nor adds a feature
Documentation: documentation changes
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

I have reviewed my own diff and added inline comments on lines I want reviewers to focus on or that I am uncertain about

stevenolen · 2026-03-05T19:54:04Z

lib/steps/postgres_config.go

-	)
+	switch s.DstTarget.CloudProvider() {
+	case types.AWS:
+		return s.runAWSInlineGo(ctx, creds, envVars)


there's likely a little dry missing here aws vs. azure, but it's probably not so important

Agreed, not important.

stevenolen · 2026-03-05T19:54:44Z

lib/steps/postgres_config.go

+
+// --- Azure ---
+
+// azurePostgresParams holds the pre-fetched data needed by the Azure deploy function.


unclear on the necessity of this, but it looks like a reasonable choice to me.

It's meh/whatever to me. Creating an options / value object to limit a giant function signature. A little easier to grok for the humans, inconsequential to the agents. 🤷

stevenolen · 2026-03-05T19:55:19Z

lib/steps/postgres_config.go

+	return runPulumi(ctx, stack, s.Options)
+}
+
+// --- Shared helpers ---


this shared helper might belong elsewhere, but it feels pretty unique to this step to me.

YAGNI. If it's not needed elsewhere yet, fine to leave in here and let the next person refactor when it needs to be used elsewhere.

stevenolen · 2026-03-05T20:01:28Z

lib/steps/postgres_config.go

+		componentType = "ptd:AWSControlRoomPostgresConfig"
+	}
+
+	// Helper to create alias pointing to old Python component parent URN.


we may want to figure out how to refactor this later to be very generic/work for all steps, since it's the real crux of how to convert these steps between languages without producing diffs/deltas

Same as above IMO: YAGNI, make it generic/reusable once there's another need for it.

stevenolen · 2026-03-05T20:02:04Z

lib/steps/postgres_config.go

+	ctx.Export("grafana_db_name", grafanaDb.Name)
+	ctx.Export("grant_id", grant.ID())
+
+	// Extra databases (workload only, e.g. soleng for staging workloads)


obviously, we shouldn't use this anymore. but i didn't want to have to clean up/migrate the existing databases prior to conversion.

Lytol

As you mentioned, there's probably some DRYing and refactoring that could make it cleaner for the next human... but definitely don't do any premature refactoring for theoretical future needs. And even the few ones that could be nice now, are probably actually irrelevant in an agentic world.

All that said, looks great to me!

Lytol · 2026-03-09T21:35:07Z

lib/pulumi/python.go

-		// the unicode word split algo also doesn't like "postgres_config", so just hack that in manually
 		titleCaseStackBaseName := helpers.TitleCase(stackBaseName)
-		if stackBaseName == "postgres_config" {
-			titleCaseStackBaseName = "PostgresConfig"
-		}


Nice to drop that nonsense.

Lytol · 2026-03-09T21:36:42Z

lib/steps/postgres_config.go

-	)
+	switch s.DstTarget.CloudProvider() {
+	case types.AWS:
+		return s.runAWSInlineGo(ctx, creds, envVars)


Agreed, not important.

Lytol · 2026-03-09T21:39:09Z

lib/steps/postgres_config.go

+
+// --- Azure ---
+
+// azurePostgresParams holds the pre-fetched data needed by the Azure deploy function.


It's meh/whatever to me. Creating an options / value object to limit a giant function signature. A little easier to grok for the humans, inconsequential to the agents. 🤷

Lytol · 2026-03-09T21:40:25Z

lib/steps/postgres_config.go

+	// Fetch DB admin secret from Azure Key Vault
+	secretName := fmt.Sprintf("%s-grafana-postgres-admin-secret", s.DstTarget.Name())
+	secretValue, err := s.DstTarget.SecretStore().GetSecretValue(ctx, creds, secretName)
+	if err != nil {
+		return fmt.Errorf("failed to get postgres admin secret from Key Vault: %w", err)
+	}
+
+	var secretData map[string]string
+	if err := json.Unmarshal([]byte(secretValue), &secretData); err != nil {
+		return fmt.Errorf("failed to parse postgres admin secret JSON: %w", err)
+	}
+
+	dbHost := secretData["fqdn"]
+	dbUser := secretData["username"]
+	dbPassword := secretData["password"]
+	if dbHost == "" || dbUser == "" || dbPassword == "" {
+		return fmt.Errorf("grafana DB secret must contain 'fqdn', 'username' and 'password' fields")
+	}


This could probably all benefit from some method extraction and encapsulation. But again, pretty nitpicky and probably inconsequential to the bots.

Lytol · 2026-03-09T21:42:17Z

lib/steps/postgres_config.go

+	return runPulumi(ctx, stack, s.Options)
+}
+
+// --- Shared helpers ---


YAGNI. If it's not needed elsewhere yet, fine to leave in here and let the next person refactor when it needs to be used elsewhere.

Lytol · 2026-03-09T21:43:22Z

lib/steps/postgres_config.go

+		componentType = "ptd:AWSControlRoomPostgresConfig"
+	}
+
+	// Helper to create alias pointing to old Python component parent URN.


Same as above IMO: YAGNI, make it generic/reusable once there's another need for it.

stevenolen · 2026-03-10T15:51:50Z

finally got around to running this in an azure env, fix a small bug (that was introduced in a go lib long ago)

⏺ Looks great. The real resources are all recognized (no creates, no deletes):

  - Provider: version bump from 3.14.0 → 3.16.2 (harmless)
  - KeyVault Secret: provider update + secret value re-evaluated (expected)
  - 2 deletes: the Python AzureWorkloadPostgresConfig and GrafanaPostgresResources component wrappers — expected since Go doesn't create those

  The postgres resources (password, role, database, grant) are all unchanged — the aliases are working correctly. This matches the same pattern we saw with the AWS previews.

@Lytol quick re-approval given last commit?

convert postgres_config steps to go

6025efd

stevenolen commented Mar 5, 2026

View reviewed changes

stevenolen requested review from Lytol and timtalbot March 5, 2026 20:04

stevenolen marked this pull request as ready for review March 5, 2026 20:05

stevenolen requested a review from a team as a code owner March 5, 2026 20:05

Lytol previously approved these changes Mar 9, 2026

View reviewed changes

fix secretstore bug

e3ef47f

stevenolen dismissed Lytol’s stale review via e3ef47f March 10, 2026 15:46

stevenolen enabled auto-merge March 10, 2026 15:51

Lytol approved these changes Mar 10, 2026

View reviewed changes

stevenolen added this pull request to the merge queue Mar 10, 2026

Merged via the queue into main with commit 6a0bc3c Mar 10, 2026
6 checks passed

stevenolen deleted the convert-step-postgres-config branch March 10, 2026 15:56


		// --- Azure ---

		// azurePostgresParams holds the pre-fetched data needed by the Azure deploy function.

Conversation

stevenolen commented Mar 5, 2026

Description

Category of change

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lytol left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevenolen commented Mar 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants