feat(connection): Support cron time zones via config API#454
feat(connection): Support cron time zones via config API#454Aaron ("AJ") Steers (aaronsteers) wants to merge 17 commits into
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Thanks for opening this pull request!Your contribution is appreciated. Here are some helpful tips and resources. 💡 Show Tips and TricksTerraform Example Commands
📚 Show Repo GuidanceAbout This RepositoryThis repository uses Speakeasy to generate the Terraform provider from the Airbyte OpenAPI specification. The CI will automatically build the provider, validate code generation, and run acceptance tests across Terraform versions 1.0-1.4. Note: This is a generated codebase. Direct modifications to generated files are not accepted - changes must be made to the upstream OpenAPI spec. |
|
Generated Code Drift Detected The committed code does not match what the generation pipeline produces. This usually means a generated file was edited by hand or the generation pipeline needs to be re-run. To fix: Comment To debug locally: Download the generated artifacts and diff them against your committed code: gh run download 26424568787 --name generated_provider_code --dir /tmp/generated_from_ci
diff -rq internal/provider/ /tmp/generated_from_ci/provider/
diff -rq --exclude=.gitattributes --exclude=.gitignore internal/sdk/ /tmp/generated_from_ci/sdk/See CONTRIBUTING.md — Debugging Generation Drift Failures for the full procedure. Diff summary |
There was a problem hiding this comment.
Pull request overview
Adds draft support for specifying an IANA cron schedule time zone on airbyte_connection by continuing to use the public Connections API for CRUD, while applying/reading scheduleData.cron.cronTimeZone via Airbyte’s internal config API.
Changes:
- Add provider-level
config_api_root(default derived fromserver_url) to target the internal config API. - Add
schedule.cron_time_zoneto connection resource and connection data source schedule shapes, and strip any legacy TZ suffix fromcron_expressionfor public API calls. - Add a reproducible post-generation patch script and bump generated SDK/provider version strings to 1.3.0.
Reviewed changes
Copilot reviewed 6 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/patch_connection_cron_timezone.py | New post-generation patcher to inject config API support + cron TZ plumbing into generated files. |
| poe_tasks.toml | Runs the new patch script as part of the generation pipeline. |
| internal/sdk/sdk.go | Bumps generated SDK version/user-agent to 1.3.0. |
| internal/provider/provider.go | Adds config_api_root to provider schema and stores derived runtime config for later config API calls. |
| internal/provider/connection_cron_timezone.go | Implements cron TZ parsing/validation, config API update/get calls, and runtime config storage. |
| internal/provider/connection_resource.go | Adds schedule.cron_time_zone schema and applies/refreshes TZ on Create/Read/Update. |
| internal/provider/connection_resource_sdk.go | Ensures public API cron expression is TZ-suffix-free and uses shared schedule apply helper. |
| internal/provider/connection_data_source_sdk.go | Uses shared schedule apply helper for data source schedule hydration. |
| internal/provider/connections_data_source_sdk.go | Uses shared schedule apply helper for list data source schedule hydration. |
| internal/provider/types/airbyte_api_connection_schedule.go | Adds cron_time_zone to the schedule type used by the resource model. |
| internal/provider/types/connection_schedule_response.go | Adds cron_time_zone to the schedule type used by data source models. |
| docs/index.md | Documents config_api_root and bumps example version to 1.3.0. |
| docs/resources/connection.md | Documents schedule.cron_time_zone on the connection resource. |
| examples/provider/provider.tf | Bumps example required provider version to 1.3.0. |
Files not reviewed (8)
- internal/provider/connection_data_source_sdk.go: Language not supported
- internal/provider/connection_resource.go: Language not supported
- internal/provider/connection_resource_sdk.go: Language not supported
- internal/provider/connections_data_source_sdk.go: Language not supported
- internal/provider/provider.go: Language not supported
- internal/provider/types/airbyte_api_connection_schedule.go: Language not supported
- internal/provider/types/connection_schedule_response.go: Language not supported
- internal/sdk/sdk.go: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var providerRuntimeConfigs sync.Map | ||
|
|
||
| func storeProviderRuntimeConfig(client *sdk.SDK, config providerRuntimeConfig) { | ||
| providerRuntimeConfigs.Store(client, config) | ||
| } | ||
|
|
||
| func getProviderRuntimeConfig(client *sdk.SDK) providerRuntimeConfig { | ||
| if config, ok := providerRuntimeConfigs.Load(client); ok { | ||
| return config.(providerRuntimeConfig) | ||
| } |
There was a problem hiding this comment.
Fixed. Runtime config is now stored on the AirbyteProvider instance and injected only into ConnectionResource; resp.ResourceData remains the generated *sdk.SDK, so there is no package-level map retaining clients.
| request.Header.Set("Accept", "application/json") | ||
| request.Header.Set("Content-Type", "application/json") | ||
| request.Header.Set("Authorization", authHeader) | ||
|
|
||
| response, err := http.DefaultClient.Do(request) |
There was a problem hiding this comment.
Fixed. providerRuntimeConfig now carries the configured *http.Client, and the config API helper uses that client rather than http.DefaultClient.
| var out configConnectionRead | ||
| if err := r.postConfigAPI(ctx, "/v1/connections/get", authHeader, body, &out); err != nil { | ||
| return diags | ||
| } |
There was a problem hiding this comment.
Fixed. Reads now preserve the previous planned/state timezone on config API refresh failure and add a warning diagnostic instead of silently clearing cron_time_zone.
| def patch_file(path: Path, patcher) -> bool: | ||
| if not path.exists(): | ||
| print(f"ERROR: {path} does not exist", file=sys.stderr) | ||
| sys.exit(1) | ||
|
|
||
| content = path.read_text() | ||
| new_content = patcher(content, path) | ||
| if new_content == content: | ||
| print(f" Skipping {path} (already patched)") | ||
| return False | ||
|
|
||
| path.write_text(new_content) | ||
| print(f" Patched {path}") | ||
| return True | ||
|
|
||
|
|
||
| def patch_provider_go(content: str, path: Path) -> str: | ||
| if "ConfigAPIRoot types.String `tfsdk:\"config_api_root\"`" not in content: | ||
| content = content.replace( | ||
| "\tPassword types.String `tfsdk:\"password\"`\n", | ||
| "\tPassword types.String `tfsdk:\"password\"`\n\tConfigAPIRoot types.String `tfsdk:\"config_api_root\"`\n", | ||
| 1, | ||
| ) | ||
| content = content.replace( | ||
| "\tBearerAuth types.String `tfsdk:\"bearer_auth\"`\n\tClientID types.String `tfsdk:\"client_id\"`\n\tClientSecret types.String `tfsdk:\"client_secret\"`\n\tPassword types.String `tfsdk:\"password\"`\n\tConfigAPIRoot types.String `tfsdk:\"config_api_root\"`\n\tServerURL types.String `tfsdk:\"server_url\"`", | ||
| "\tBearerAuth types.String `tfsdk:\"bearer_auth\"`\n\tClientID types.String `tfsdk:\"client_id\"`\n\tClientSecret types.String `tfsdk:\"client_secret\"`\n\tPassword types.String `tfsdk:\"password\"`\n\tConfigAPIRoot types.String `tfsdk:\"config_api_root\"`\n\tServerURL types.String `tfsdk:\"server_url\"`", | ||
| 1, | ||
| ) | ||
|
|
||
| if '"config_api_root": schema.StringAttribute{' not in content: |
There was a problem hiding this comment.
Fixed. The patch script now uses fail-fast helper functions for expected replacements and reports an error when an anchor drifts, rather than treating no-op patch results as success.
| content = content.replace( | ||
| "\tPassword types.String `tfsdk:\"password\"`\n", | ||
| "\tPassword types.String `tfsdk:\"password\"`\n\tConfigAPIRoot types.String `tfsdk:\"config_api_root\"`\n", | ||
| 1, | ||
| ) | ||
| content = content.replace( | ||
| "\tBearerAuth types.String `tfsdk:\"bearer_auth\"`\n\tClientID types.String `tfsdk:\"client_id\"`\n\tClientSecret types.String `tfsdk:\"client_secret\"`\n\tPassword types.String `tfsdk:\"password\"`\n\tConfigAPIRoot types.String `tfsdk:\"config_api_root\"`\n\tServerURL types.String `tfsdk:\"server_url\"`", | ||
| "\tBearerAuth types.String `tfsdk:\"bearer_auth\"`\n\tClientID types.String `tfsdk:\"client_id\"`\n\tClientSecret types.String `tfsdk:\"client_secret\"`\n\tPassword types.String `tfsdk:\"password\"`\n\tConfigAPIRoot types.String `tfsdk:\"config_api_root\"`\n\tServerURL types.String `tfsdk:\"server_url\"`", | ||
| 1, |
There was a problem hiding this comment.
Fixed. The remaining patchers were moved onto checked replacement helpers so missing anchors fail the generation patch step instead of producing partial output.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| return content.replace( | ||
| '''"cron_expression": schema.StringAttribute{ | ||
| Computed: true, | ||
| },''', | ||
| '''"cron_expression": schema.StringAttribute{ | ||
| Computed: true, | ||
| }, | ||
| "cron_time_zone": schema.StringAttribute{ | ||
| Computed: true, | ||
| },''', | ||
| 1, | ||
| ) |
There was a problem hiding this comment.
🔴 Data source schemas missing cron_time_zone attribute, causing runtime errors
The ConnectionScheduleResponse struct has CronTimeZone types.String \tfsdk:"cron_time_zone"`added atinternal/provider/types/connection_schedule_response.go:12, but neither connection_data_source.gonorconnections_data_source.godeclarescron_time_zone in their schedule schema (connection_data_source.go:259-268, connections_data_source.go:258-267). The Terraform Plugin Framework strictly validates that every tfsdk-tagged struct field has a corresponding schema attribute. When resp.State.Set(ctx, &data)is called with aConnectionScheduleResponsecontainingCronTimeZone, the framework will return an error diagnostic because cron_time_zonedoesn't exist in the schema. This breaks ALL reads of both theairbyte_connectionandairbyte_connections` data sources.
Root cause in patch script
The patch_data_source_schema function in scripts/patch_connection_cron_timezone.py:196-198 searches for a pattern with no tab indentation ("cron_expression": schema.StringAttribute{\nComputed: true,\n},), but the actual generated files use tabs. Python's str.replace silently returns the original string when no match is found, so the patch appears to succeed but does nothing.
Prompt for agents
The patch_data_source_schema function in scripts/patch_connection_cron_timezone.py uses unindented search patterns that never match the tab-indented content of the generated data source files (connection_data_source.go and connections_data_source.go). The function silently does nothing because Python's str.replace returns the original string when no match is found.
The search pattern at lines 196-198 has no indentation:
'"cron_expression": schema.StringAttribute{\nComputed: true,\n},'
But the actual files use tab indentation, e.g. in connection_data_source.go:
'\t\t\t\t\t"cron_expression": schema.StringAttribute{\n\t\t\t\t\t\tComputed: true,\n\t\t\t\t\t},'
and in connections_data_source.go (deeper nesting):
'\t\t\t\t\t\t\t\t"cron_expression": schema.StringAttribute{\n\t\t\t\t\t\t\t\t\tComputed: true,\n\t\t\t\t\t\t\t\t},'
The fix needs to either: (a) use the correctly indented search patterns for each file (note: each data source file has different indentation depth), or (b) use a regex-based approach that handles varying indentation. Since the two data source files have different indentation levels, using a single shared patch_data_source_schema function with a hardcoded pattern won't work for both. Consider splitting into separate patchers or using regex with flexible whitespace matching.
After fixing the script, re-run it to update connection_data_source.go and connections_data_source.go so they include the cron_time_zone schema attribute.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed. The data-source schedule schemas now include cron_time_zone, matching the added ConnectionScheduleResponse.CronTimeZone field so resp.State.Set has a schema attribute for the value.
| resp.Diagnostics.Append(data.RefreshFromSharedConnectionResponse(ctx, res.ConnectionResponse)...) | ||
| resp.Diagnostics.Append(r.applyCronTimeZone(ctx, data, res.ConnectionResponse.ConnectionID, res.RawResponse)...) |
There was a problem hiding this comment.
🔴 applyCronTimeZone reads plan's CronTimeZone after RefreshFromSharedConnectionResponse has overwritten it to null
In both Create and Update flows, RefreshFromSharedConnectionResponse is called before applyCronTimeZone. The refresh calls applyCronScheduleResponse(r.Schedule, resp.Schedule.CronExpression, nil) (connection_resource_sdk.go:148), which sets schedule.CronTimeZone = types.StringPointerValue(nil) (null). Then applyCronTimeZone calls cronScheduleParts(data.Schedule) (connection_cron_timezone.go:166), which checks !schedule.CronTimeZone.IsNull() — but it IS null, so cronTimeZone stays "", and the function returns early without ever calling the config API. The user's cron_time_zone value is never applied to the server. Although refreshPlan later restores the plan value into state, the server-side timezone remains at the default (UTC), causing perpetual drift on every subsequent plan/apply cycle.
Prompt for agents
In connection_resource.go, the Create flow (lines 806-807) and Update flow (lines 922-923), applyCronTimeZone is called after RefreshFromSharedConnectionResponse, which overwrites data.Schedule.CronTimeZone to null. The applyCronTimeZone function then reads the null CronTimeZone and returns early without calling the config API.
The fix should preserve the user's planned CronTimeZone value before RefreshFromSharedConnectionResponse overwrites it, then pass it to applyCronTimeZone. One approach:
1. Before calling RefreshFromSharedConnectionResponse, save the planned timezone:
var plannedCronTimeZone string
if data.Schedule != nil {
plannedCronTimeZone = data.Schedule.CronTimeZone.ValueString()
}
2. Modify applyCronTimeZone to accept the planned timezone as a parameter instead of reading it from data.Schedule.CronTimeZone.
Alternatively, change cronScheduleParts to accept an explicit timezone string parameter rather than reading from the schedule struct.
This needs to be fixed in both the Create flow (around line 806) and the Update flow (around line 922). Since these are generated files patched via scripts/patch_connection_cron_timezone.py, the actual fix should be applied in the patch script (patch_connection_resource_go function) and in the hand-written connection_cron_timezone.go file.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed. Create and Update now preserve the planned cron_time_zone before RefreshFromSharedConnectionResponse resets generated schedule fields, then pass that preserved value into applyCronTimeZone.
Summary
Adds draft support for real IANA cron time zones on
airbyte_connectionby using Airbyte's internal/config API for the schedule timezone field that the public Connections API does not expose.Key findings:
scheduleTypeandcronExpression; public create/update mappers do not expose a timezone field.scheduleData.cron.cronTimeZone, and Airbyte workers evaluate schedules using that IANA timezone./api/v1/connections/updateand/api/v1/connections/get.Changes in this PR:
config_api_root, defaulting fromserver_url.schedule.cron_time_zonefor connection resources and connection data-source schedule shapes.cron_expression.ProviderDatacompatible with*sdk.SDK.Review & Testing Checklist for Human
schedule = { schedule_type = "cron", cron_expression = "0 0 12 * * ?", cron_time_zone = "America/New_York" }and verify the Airbyte connection storesscheduleData.cron.cronTimeZone = "America/New_York".config_api_rootin Cloud, especially whether a public API bearer token can callhttps://cloud.airbyte.com/api/v1/connections/update.cron_time_zoneschema attribute is present.Notes
Local verification performed:
python3 scripts/patch_connection_cron_timezone.pyuvx --from=poethepoet poe post-generategenerated_provider_codeartifact from run26424475179and matchedinternal/provider+internal/sdkwithdiff -rqgo build ./...go test ./...poe -C /home/ubuntu/repos/terraform-provider-airbyte docs-generateEarlier CI verification passed all PR checks before the review-fix commits, including build, docs drift, generated-code zero-diff, E2E smoke test, and Terraform acceptance matrix. Latest CI is pending after the zero-diff alignment fix.
This is intentionally a draft because the implementation depends on whether calling internal/config APIs from the provider is product/API-supported.
Requested by: Aaron ("AJ") Steers (@aaronsteers)
Link to Devin session: https://app.devin.ai/sessions/76466ef911ea4604b7be6d5a055e5f7c