feat(workflows): Onboarding STACKIT Workflows#1528
Conversation
Adds Terraform support for STACKIT Workflows (Apache Airflow 3 as a
service):
resources: stackit_workflows_instance, stackit_workflows_dag_bundle
data sources: stackit_workflows_instance, stackit_workflows_instances,
stackit_workflows_dag_bundle, stackit_workflows_dag_bundles,
stackit_workflows_provider_options
All seven surfaces are gated behind the `workflows` experiment flag.
Notes for reviewers:
* The workflows Go SDK is not yet on a published tag; this PR uses a
replace-directive in `go.mod` pointing at the bot-generated branch.
This is per the dev-tools team's request to enable an early review
pass. The replace will be dropped once the SDK release lands.
* `dag_bundle` list/get endpoints currently return 403 in prod for
service-account-authenticated callers. The server-side fix is merged
upstream and awaiting deploy; acceptance tests against those endpoints
will only pass once that ships.
* A separate cluster-side issue (helm chart unconditionally provisions
an ExternalSecret for the git-bundle credentials) means Airflow 3
instances cannot reach `active` unless a dag_bundle is configured at
Create time. This is owned by the API team. Subsequent provider PR
will inline dag_bundles onto the instance Create payload to work
around it cleanly.
|
This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it. |
| "region": schema.StringAttribute{Description: schemaDescriptions["region"], Optional: true, Computed: true}, | ||
| "instance_id": schema.StringAttribute{ | ||
| Description: schemaDescriptions["instance_id"], | ||
| Required: true, |
There was a problem hiding this comment.
Please sort: required - optional here please
| return | ||
| } | ||
| d.client = apiClient | ||
| } |
There was a problem hiding this comment.
tflog.Info() message missing here (... client configured...)
| return | ||
| } | ||
| resp.Diagnostics.Append(resp.State.Set(ctx, model)...) | ||
| tflog.Debug(ctx, "Workflows DAG bundle read") |
There was a problem hiding this comment.
| tflog.Debug(ctx, "Workflows DAG bundle read") | |
| tflog.Info(ctx, "Workflows DAG bundle read") |
|
|
||
| bundle, err := d.client.DefaultAPI.GetDagBundle(ctx, projectID, region, instanceID, name).Execute() | ||
| if err != nil { | ||
| var oapiErr *oapierror.GenericOpenAPIError |
There was a problem hiding this comment.
This block "http.StatusNotFound" can be put into the tfutils.LogError() -> can be handled there.
| if resp.Diagnostics.HasError() { | ||
| return | ||
| } | ||
| r.client = apiClient |
| } | ||
| list, diags := types.ListValue(objType, elements) | ||
| if diags.HasError() { | ||
| return fmt.Errorf("%v", diags.Errors()) |
There was a problem hiding this comment.
same here: core.DiagsToError(diags)
| // requireEnv collects a set of required env vars for these tests; if any are | ||
| // missing, skips the test with a clear message. Keeps test runs cheap when | ||
| // workflows-specific secrets aren't provisioned in the runner. | ||
| func requireEnv(t *testing.T, keys ...string) map[string]string { |
There was a problem hiding this comment.
I like this function but thought more about checking the general env variables here like TF_ACC_PROJECT_ID
| "TF_ACC_WORKFLOWS_IDP_CLIENT_ID", | ||
| "TF_ACC_WORKFLOWS_IDP_CLIENT_SECRET", | ||
| "TF_ACC_WORKFLOWS_IDP_SCOPE", | ||
| "TF_ACC_WORKFLOWS_IDP_DISCOVERY_ENDPOINT", |
There was a problem hiding this comment.
Our acceptance tests are structured as following:
All needed information and additional instances should be defined in the acceptance test. There should be no need to set special things beforehand or creating other instances in a manual step. You can have a look at other acceptance tests. Besides some general environment variables like PROJECT_ID or ORGANIZATION_ID (+ some more) no special values are passed to an acceptance test.
Another thing: We have a Test...Min and a Test...Max for all those tests (where it makes sense of course).
Min test: only the required parameters are set in order to test that this is working as expected.
Max test: all parameters are set (to one example value if "a lot" of values would be possible).
| "TF_ACC_WORKFLOWS_DAGS_GIT_URL", | ||
| "TF_ACC_WORKFLOWS_DAGS_GIT_BRANCH", | ||
| "TF_ACC_WORKFLOWS_DAGS_GIT_USER", | ||
| "TF_ACC_WORKFLOWS_DAGS_GIT_PAT", |
There was a problem hiding this comment.
Same here: I don't want to pass them before running the acceptance test
| ) | ||
|
|
||
| replace github.com/stackitcloud/stackit-sdk-go/services/workflows => /Users/cht/code/schwarz/stackit-sdk-go/services/workflows | ||
|
|
There was a problem hiding this comment.
Just a reminder: Replace before merging
Adds Terraform support for STACKIT Workflows (Apache Airflow 3 as a service):
All seven surfaces are gated behind the
workflowsexperiment flag.Notes for reviewers:
go.modpointing at the bot-generated branch. This is per the dev-tools team's request to enable an early review pass. The replace will be dropped once the SDK release lands.dag_bundlelist/get endpoints currently return 403 in prod for service-account-authenticated callers. The server-side fix is merged upstream and awaiting deploy; acceptance tests against those endpoints will only pass once that ships./dag-bundlesresource. Two follow-up items are still in flight and are not provider bugs:/dag-bundleslist/get endpoints currently return 403 forservice-account callers in production. The fix is merged upstream
and awaiting deploy; acceptance tests against those endpoints will
pass once it ships.
ExternalSecret for the DAG-bundle git credentials, which means an
Airflow 3 instance created without a bundle can't reach
active.The team is working on this; in the meantime a follow-up provider
PR will inline the bundles back onto the instance Create payload
(which the API already supports) to side-step it.
Description
relates to #1234
Checklist
make fmtexamples/directory)make generate-docs(will be checked by CI)make test(will be checked by CI)make lint(will be checked by CI)