Skip to content

refactor: replace custom HTTP client with Go SDK#4

Open
Leechael wants to merge 12 commits intomainfrom
refactor/use-go-sdk
Open

refactor: replace custom HTTP client with Go SDK#4
Leechael wants to merge 12 commits intomainfrom
refactor/use-go-sdk

Conversation

@Leechael
Copy link
Copy Markdown

Summary

  • Replace the provider's custom HTTP client layer (client.go, client_typed.go) and oapi-codegen generated client (internal/phalaapi/) with the Phala Cloud Go SDK
  • All API calls now use typed SDK methods (ProvisionCVM, GetCVMInfo, PatchCVM, etc.) instead of raw PostJSON/GetJSON/PatchJSON
  • Removed ~37k lines of duplicated/generated API client code
  • Bumped Go version to 1.25.0
  • Removed oapi-codegen/runtime dependency

Test plan

  • go build ./... passes
  • go vet ./... passes
  • go test ./... passes (all existing tests updated to use SDK types)
  • Manual terraform plan / terraform apply against staging environment
  • Verify CVM create, read, update, delete lifecycle
  • Verify SSH key management
  • Verify data sources (account, workspace, sizes, regions, images, nodes, attestation)

Replace the Terraform provider's custom HTTP client layer (client.go,
client_typed.go) and oapi-codegen generated client (internal/phalaapi/)
with the Phala Cloud Go SDK. This eliminates ~37k lines of duplicated
API client code and ensures both projects evolve together.

Key changes:
- All API calls now use phala.Client methods (ProvisionCVM, GetCVMInfo,
  PatchCVM, DeleteCVM, etc.) instead of raw PostJSON/GetJSON/PatchJSON
- cvmAPIResponse replaced with phala.CVMInfo from SDK
- authMeResponse replaced with phala.CurrentUser from SDK
- Data sources use typed SDK responses (AvailableNodes, CVMAttestation)
- Removed oapi-codegen dependency and OpenAPI specs
- Bumped Go version to 1.25.0
- Added local replace directive for Go SDK (dev only)
Leechael added a commit to Phala-Network/phala-cloud that referenced this pull request Mar 18, 2026
…gration

Enhance Go SDK types to support Terraform provider's needs and update
the terraform submodule to use the Go SDK instead of its custom HTTP
client layer.

Go SDK changes:
- CVMInfo: add InProgress, Endpoints, PublicURLs, DiskSize, EncryptedEnvPubkey
- SSHKey: add Fingerprint, KeyType, Source, UpdatedAt
- UserInfo/CreditsInfo: use pointer types for nullable field handling
- ProvisionCVMRequest: add SSHAuthorizedKeys, CustomAppID, Nonce, StorageFS
- CommitCVMProvisionRequest: add EncryptedEnv, EnvKeys
- ProvisionComposeUpdateRequest: add Name, visibility flags, UpdateEnvVars

Terraform submodule updated to refactor/use-go-sdk branch
(Phala-Network/terraform-provider-phala#4).
The Go SDK module path (github.com/Phala-Network/phala-cloud/sdks/go)
doesn't match the repo directory structure (go/ not sdks/go/), so
go mod download can't resolve it. Use a local replace directive and
checkout the SDK in CI workflows.
Comment thread internal/provider/resource_shared.go
Add diagnosticFromAPIError() helper that extracts structured error
details (error_code, suggestions, links) from Go SDK's APIError
into Terraform-friendly diagnostic messages. Update ProvisionCVM,
commitAndCreate, and env update error paths in resource_app.go.
Comment thread internal/provider/cvm_helpers.go
Comment thread internal/provider/resource_app.go Outdated
- CI: checkout refactor/terraform-use-go-sdk branch of phala-cloud for
  SDK type enhancements
- buildProvisionReq: add missing public_logs, public_sysinfo,
  public_tcbinfo, secure_time to ComposeFile during provisioning
- diagnosticFromAPIError: remove dependency on IsStructured/FormatError
  methods not yet on SDK main
Comment thread internal/provider/resource_shared.go Outdated
- buildProvisionReq: set Region on ProvisionCVMRequest instead of
  discarding it, so CVMs deploy to the user-specified region
- populateState: only update Listed from API when explicitly true,
  preventing spurious RequiresReplace drift when API omits the field
@Leechael
Copy link
Copy Markdown
Author

Addressed the review comments:

buildProvisionReq missing compose fields (sentry comment #1): Fixed in 992daca + acd8552. Added public_logs, public_sysinfo, public_tcbinfo, secure_time to ComposeFile construction. Also added these fields to the SDK's ComposeFile struct.

Resource nil dereference (sentry comment #2): Not a real issue — info.Resource is a value type (CvmResource, not a pointer), so it's always initialized. No nil dereference possible.

Listed drift (sentry comment #3): Fixed in acd8552. Only update state.Listed from API when explicitly true, preventing spurious RequiresReplace drift when the API omits the field (Go zero-value false is ambiguous).

Region silently discarded: Fixed in acd8552. Added Region field to SDK's ProvisionCVMRequest and wired it through in buildProvisionReq.

Comment thread internal/provider/resource_app.go Outdated
Comment thread internal/provider/resource_app.go
Comment thread internal/provider/resource_app.go
- Use ReplicateAppCVM (POST /apps/{appID}/cvms/{vmUUID}/replicas)
  instead of ReplicateCVM to maintain correct app association
- Fix trailing newline in provider.go (gofmt)
- Handle CreditsInfo fields changed to *StringOrNumber in SDK
@Leechael
Copy link
Copy Markdown
Author

Addressed the 3 new review comments:

ReplicateCVM endpoint (comment #4): Fixed in b7b4581. Added ReplicateAppCVM to the SDK that uses the app-scoped endpoint POST /apps/{appID}/cvms/{vmUUID}/replicas. Terraform now uses this instead of the CVM-scoped ReplicateCVM.

Resource nil check on DiskInGB (comment #5): Not a real issue — CVMInfo.Resource is CvmResource (value type, not pointer), always initialized by Go. No nil dereference possible.

Listed only updates on true (comment #6): By design. Listed is bool (not *bool) in the SDK, so Go zero-value false is indistinguishable from "API omitted the field". Since Listed has RequiresReplace, a false positive (overwriting with false when API didn't return it) would destructively recreate the resource. The current behavior is the safer choice.

The refactor/terraform-use-go-sdk branch was merged into
Phala-Network/phala-cloud main and deleted, causing the sparse
checkout step to fail with a 404.
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.

1 participant