Skip to content

Migrate CLI to v1 public API; retire v1beta1 and v1alpha1 (except IDE)#2093

Open
jeremybeard wants to merge 6 commits into
mainfrom
remove-v1beta1
Open

Migrate CLI to v1 public API; retire v1beta1 and v1alpha1 (except IDE)#2093
jeremybeard wants to merge 6 commits into
mainfrom
remove-v1beta1

Conversation

@jeremybeard
Copy link
Copy Markdown
Contributor

@jeremybeard jeremybeard commented Apr 17, 2026

Summary

Retires the astro-client-core (v1alpha1), astro-client-iam-core (iam/v1beta1), and astro-client-platform-core (platform/v1beta1) generated clients in favor of a single unified astro-client-v1 client built from the public versioned/v1.0 spec released in astronomer/astro#37986.

  • New: astro-client-v1/ — unified client, tags for Organization/Workspace/Cluster/Deployment/Deploy/User/Team/ApiToken/Environment/Authorization/etc.
  • Renamed: astro-client-core/astro-client-v1alpha1/, shrunk to IDE-only (Polaris Project, Polaris Session tags).
  • Deleted: astro-client-iam-core/, astro-client-platform-core/, cloud/platformclient/.
  • Two endpoints stay on v1alpha1 because the server has no v1 equivalent: private/v1alpha1/cli/auth-config (pkg/astroauth/config.go) and /v1alpha1/telemetry (pkg/telemetry/telemetry.go).
  • Legacy tests (22 files) fully migrated; dead cmd-layer test paths covering collapsed split-client APIs were dropped (service-layer coverage remains).

Notable production fixes surfaced during migration

  • cloud/deployment/inspect/inspect.go: format DeploymentEnvironmentVariable.UpdatedAt as RFC3339 before mapstructure decode (v1 changed the field from string to time.Time; inspect was broken for any deployment with env vars).
  • cmd/cloud/deployment.go: uppercase --cloud-provider input before validating against the ClusterCloudProvider enum (AWS/GCP/AZURE) so lowercase input keeps working.
  • cloud/deploy/bundle.go: wire v1 CreateDeployGitRequest (GITHUB + GENERIC providers) into CreateDeployRequest.Git now that the v1 spec exposes it.
  • cloud/deployment/deployment.go + cloud/deployment/fromfile/fromfile.go: stop synthesizing client-side defaults for optional deployment fields (scheduler size, hybrid scheduler AU/replicas, default worker queue, task-pod node pool). The server applies defaults; sending them from the CLI caused schema drift.
  • scripts/patch-v1-gen/ (Go): function-scope-aware rewrite of oneOf discriminator setters that oapi-codegen (≤ v2.6.0) emits with untyped string literals into pointer fields.

Test plan

  • go build ./... clean
  • make lint clean
  • go test ./... passes
  • Live smoke against local astro (localhost context, organization Sample Organization, workspace Sample Workspace, cluster Sample AWS Cluster):
    • astro login localhost — v1 /users/self
    • astro organization list / astro workspace list
    • astro deployment list, astro deployment inspect <id> (full + --key), astro deployment logs <id>
    • astro deployment create --type dedicated — confirmed server applies defaults (scheduler size MEDIUM, default worker queue, task-pod cpu/memory) that the CLI no longer synthesizes
    • astro deployment update --description … — inspect confirms change
    • astro deployment inspect --templateastro deployment create --deployment-file (fromfile create), then astro deployment update --deployment-file (fromfile update); env-var updated_at decoded correctly (RFC3339 mapstructure fix)
    • astro deployment variable create/update/list (including --secret)
    • astro deployment worker-queue create/update/delete
    • astro deployment token create/list/update/rotate/delete (identified by -t <name>)
    • astro workspace token create/list/update/rotate/delete
    • astro organization token create/list/update/rotate/delete
    • astro organization team create/list/delete; astro workspace team add/update/list; astro deployment team add/update/list — exercises the new workspaceId / deploymentId query params on GET /organizations/{orgId}/teams
    • astro organization user list/invite/update; astro workspace user add/update/remove/list; astro deployment user add/update/remove/list
    • astro deployment delete — cleaned up both smoke deployments

Retires the astro-client-core (v1alpha1), astro-client-iam-core
(iam/v1beta1), and astro-client-platform-core (platform/v1beta1)
generated clients in favor of a single unified astro-client-v1 built
from the public versioned/v1.0 spec released in astronomer/astro#37986.

- New: astro-client-v1/ — unified client with Organization, Workspace,
  Cluster, Deployment, Deploy, User, Team, ApiToken, Environment,
  Authorization, and other tags.
- Renamed: astro-client-core/ → astro-client-v1alpha1/, shrunk to
  IDE-only (Polaris Project, Polaris Session).
- Deleted: astro-client-iam-core/, astro-client-platform-core/,
  cloud/platformclient/.
- Two endpoints stay on v1alpha1 because the server has no v1
  equivalent: private/v1alpha1/cli/auth-config and /v1alpha1/telemetry.

Structural adaptations required by v1:
- ApiToken: scope is carried on the token itself; per-scope CRUD
  endpoints collapse to CreateApiToken / UpdateApiToken /
  UpdateApiTokenRoles / RotateApiToken / DeleteApiToken. Token creation
  is now a two-step CreateApiToken + UpdateApiTokenRoles flow.
- Users/teams: role mutations go through UpdateUserRoles /
  UpdateTeamRoles with full-role-set replacement.
- Deployment create/update: most optional fields are pointer-typed;
  wrappers absorb the shape changes. Client-side defaults for optional
  fields (scheduler size, hybrid scheduler AU/replicas, default worker
  queue, task-pod node pool) are removed — the server applies defaults.
- Deployment: WorkloadIdentity → EffectiveWorkloadIdentity.
- Deploys: UpdateDeploy(BundleTarballVersion) → FinalizeDeploy.
- Deploy git metadata: v1 CreateDeployGitRequest wired in with GITHUB
  and GENERIC provider branches (remoteUrl for non-GitHub SSH remotes).

Other cleanups in scope:
- Use Go 1.26 new() builtin for pointer init; drop the ptrOf helper.
- Drop 'core client' vocabulary from the live tree; rename type alias
  CoreClient → APIClient; use astroV1Client throughout.
- Upgrade oapi-codegen to v2.6.0 and replace the shell patch with a
  function-scope-aware Go program at scripts/patch-v1-gen/ that rewrites
  the remaining oneOf discriminator setter bug.

Production fixes surfaced during migration:
- cloud/deployment/inspect/inspect.go: format
  DeploymentEnvironmentVariable.UpdatedAt as RFC3339 before mapstructure
  decode (v1 changed the field from string to time.Time; inspect was
  broken for any deployment with env vars).
- cmd/cloud/deployment.go: uppercase --cloud-provider input before
  validating against the ClusterCloudProvider enum so lowercase input
  keeps working.
@jeremybeard jeremybeard marked this pull request as ready for review April 22, 2026 18:28
@jeremybeard jeremybeard requested review from a team as code owners April 22, 2026 18:28
Copy link
Copy Markdown

@akshayarora akshayarora left a comment

Choose a reason for hiding this comment

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

Thanks for the migration work. Left one comment.

Comment thread cloud/deployment/deployment.go Outdated
} else {
developmentModeValue = false
}
var workerConcurrency int
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

im assuming this deletion is intentional?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, very recently the create deployment API was changed to make the worker queues optional, so dropping this logic. There's more we could do later here to have the server apply even more defaults.

WorkloadIdentity: deplWorkloadIdentity,
RemoteExecution: remoteExecution,
}
if !remoteExecutionEnabled {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

im assuming this deletion is intentional?

WorkloadIdentity: deplWorkloadIdentity,
RemoteExecution: remoteExecution,
}
if !remoteExecutionEnabled {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

im assuming this deletion is intentional?

@aliotta
Copy link
Copy Markdown
Contributor

aliotta commented Apr 23, 2026

I wonder how difficult it would be to wire up some component type integration tests that use the cli such that refactors like this do not require the full suite of manual tests you had to run.

@aliotta aliotta self-requested a review May 21, 2026 20:36
if workloadIdentity == "" {
if currentDeployment.WorkloadIdentity != nil {
workloadIdentity = *currentDeployment.WorkloadIdentity
if currentDeployment.EffectiveWorkloadIdentity != nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

heads up, the swap from WorkloadIdentity to EffectiveWorkloadIdentity is forced (v1's Deployment only exposes the effective one, per the field doc: "either user-configured or Astronomer-provided default"), but it does shift behavior in this fallback path

previously, if the deployment had no user-configured identity, currentDeployment.WorkloadIdentity was nil and we skipped the assignment, so the Update sent no identity and the server kept managing it. now, EffectiveWorkloadIdentity is non-nil whenever a default is in effect, so an Update without --workload-identity on a previously-server-managed deployment will stamp the Astronomer-provided default as the requested value going forward

is that intentional? inspect.go:279-280 has the matching shift so the inspect template also bakes the effective identity in, which means inspect --template -> create --deployment-file round-trips pin the default too

longer-term, probably worth flagging back to the api team that the read model could expose the user-configured identity separately (or a flag indicating whether the effective value was user-set vs defaulted) so we can restore the prior nil-means-skip behavior. either way a short code comment noting the semantic difference would help future readers

Copy link
Copy Markdown
Contributor

@jlaneve jlaneve left a comment

Choose a reason for hiding this comment

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

lgtm, awesome cleanup. left one non-blocking note re the EffectiveWorkloadIdentity semantics shift, can be followed up separately

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants