Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(accounts): fix the account domains payload #393

Merged
merged 2 commits into from
Feb 28, 2025

Conversation

mitchnielsen
Copy link
Contributor

@mitchnielsen mitchnielsen commented Feb 28, 2025

Summary

Fixes the payload type of account domains.

The API docs don't show the structure of the response, so we mistakenly assumed it was the same as the request payload - a list of strings.

It's actually a list of objects, and the name is one of the fields in those objects.

So this change accounts for that structure, and extracts the names from it.

Follow-up to #382

Closes #390

Testing

You can't create accounts with Terraform, and to test SSO it has to be enabled on a paid account, so I used the existing github-ci-tests account we have in staging:

terraform {
  required_providers {
    prefect = {
      source = "registry.terraform.io/prefecthq/prefect"
    }
  }
}

provider "prefect" {
  endpoint = "https://api.stg.prefect.dev"
  account_id = "bb19c492-73c2-4ecd-9cd7-d82c4aac08e6"
  api_key = "<redacted>"
}

resource "prefect_account" "test" {
  name   = "github-ci-tests"
  handle = "github-ci-tests"
  # Added "update.com"
  domain_names = ["example.com", "foobar.com", "update.com"]
  settings = {
    ai_log_summaries        = true
    allow_public_workspaces = false
    managed_execution       = true
  }
}

Per the comment, I added update.com to my Terraform (the first two existed already because I manually set them to satisfy the datasource acceptance test).

I applied the change and it added the value as expected:

Terraform will perform the following actions:

  # prefect_account.test will be updated in-place
  ~ resource "prefect_account" "test" {
      ~ domain_names = [
            # (1 unchanged element hidden)
            "foobar.com",
          + "update.com",
        ]
        id           = "bb19c492-73c2-4ecd-9cd7-d82c4aac08e6"
        name         = "github-ci-tests"
      ~ updated      = "2025-02-28T22:41:58Z" -> (known after apply)
        # (3 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

prefect_account.test: Modifying... [id=bb19c492-73c2-4ecd-9cd7-d82c4aac08e6]
prefect_account.test: Modifications complete after 1s [id=bb19c492-73c2-4ecd-9cd7-d82c4aac08e6]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.
image

This covers the resource testing. The datasource testing, as mentioned, is part of the acceptance test suite as of this PR.

Requirements

General

  • The contributing guide has been read
  • Title follows the conventional commits format
  • Body includes Closes <issue>, if available
  • Relevant labels have been added
  • Draft status is used until ready for review

Code-level changes

  • Unit tests are added/updated
  • Acceptance tests are added/updated (including import tests, when needed)

New or updated resource/datasource

  • Documentation is added (generated by make docs from source code)
    - When applicable, provide a link back to the relevant page in the Prefect documentation site.
  • For resources, the following are added:
    - Resource example under examples/resources/prefect_<name>/resource.tf
    - Import example under examples/resources/prefect_<name>/import.sh
  • For datasources, the following is added:
    - Datasource example under examples/data-sources,resources>/prefect_<name>/data-source.tf

Fixes the payload type of account domains.

The API docs don't show the structure of the response, so we mistakenly
assumed it was the same as the request payload - a list of strings.

It's actually a list of objects, and the name is one of the fields in
those objects.

So this change accounts for that structure, and extracts the names from
it.

Closes #390
We want to pass the entire payload, not just the sub-element.
@mitchnielsen mitchnielsen merged commit 45b3102 into main Feb 28, 2025
7 checks passed
@mitchnielsen mitchnielsen deleted the fix-account-domains branch February 28, 2025 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixing a bug docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plan/Apply fails on 2.17.1 with prefect_account data source
3 participants