Skip to content

feature(remote): improved tfe service discovery#254

Open
gringolito wants to merge 2 commits intofujiwara:mainfrom
gringolito:feature/tfstate-remote-service-discovery
Open

feature(remote): improved tfe service discovery#254
gringolito wants to merge 2 commits intofujiwara:mainfrom
gringolito:feature/tfstate-remote-service-discovery

Conversation

@gringolito
Copy link
Copy Markdown

@gringolito gringolito commented Jul 7, 2025

This commit enhances the remote support by implementing proper service discovery using Terraform's disco package to initialize the TFE client. This adds supports for all implementations of the Terraform Remote Backend (such as JFrog's Artifactory) and follows HashiCorp's service discovery patterns.

This commit enhances the `tfstateremote` support by implementing proper
service discovery using Terraform's disco package to initialize the TFE
client. This adds supports for all implementations of the Terraform
Remote Backend (such as JFrog's Artifactory) and follows HashiCorp's
service discovery patterns.

Signed-off-by: Filipe Utzig <filipe@gringolito.com>
@gringolito gringolito force-pushed the feature/tfstate-remote-service-discovery branch from 1f899af to 38fd428 Compare July 7, 2025 20:08
@gringolito
Copy link
Copy Markdown
Author

@fujiwara Just checking in to see if you’ve had a chance to review the PR. I’d appreciate any feedback or thoughts you might have when you have a moment.

@fujiwara
Copy link
Copy Markdown
Owner

@gringolito Thanks for the PR! I’m a bit busy at the moment, but I’ll take a look and provide feedback this weekend.

@fujiwara
Copy link
Copy Markdown
Owner

fujiwara commented Jul 19, 2025

@gringolito

First, this PR seems to work fine on my environment(Terraform Cloud).

But I don't have other environment like Terraform Enterprise or JFrog's Artifactory.
Could you please show me the test results on these environments if you can?

Second, terraform-svchost seems to print debug logs always as below. (May be here)

$ tfstate-lookup -s remote://app.terraform.io/example-org-foobar/getting-started
2025/07/19 21:26:09 [DEBUG] Service discovery for app.terraform.io at https://app.terraform.io/.well-known/terraform.json
...

This is not good because tfstate-lookup a library embedded into other products, for example ecspresso, helmfile/vals and etc.

Once these issues are resolved, I'm planning to accept this PR.

@gringolito
Copy link
Copy Markdown
Author

@fujiwara

Thanks for the review.

I've noticed the debug logs here too, but thought that was something that I had enabled locally, since this library is used by the Terraform project, let me find out how to disable them (on the Terraform binary this message is not printed to the stdout by default).

About the Artifactory test, let me find if I can setup a bucket anywhere public where you can verify it yourself, but so far I've been using it successfully with Artifactory and Terraform Remote backend.

@gringolito
Copy link
Copy Markdown
Author

@fujiwara

I’ve been exploring a few options to address the logging behavior in the svchost package, and I’d really appreciate your input:

  1. Configure logging in the tfstate-lookup application to write logs to a file instead of stdout. This would resolve the issue for the tfstate-lookup binary specifically, but log messages would still be printed to stdout for any other consumers, unless they implement similar logging configuration themselves.

  2. Configure logging within the tfstate library package to write to a file. This would affect both tfstate-lookup and other clients using the library. However, this feels a bit too invasive to me, as it changes application behavior at the library level. In my view, this library shouldn’t be responsible for setting up logging infrastructure.

  3. Switch to the opentofu/svchost package, which avoids printing log messages by default. I’ve already tested it against Artifactory, and it behaves the same as the HashiCorp version, without the logging noise.

I'm currently leaning toward option 3, but I’d like to hear your thoughts before moving forward.

@fujiwara
Copy link
Copy Markdown
Owner

@gringolito

Thank you for exploring the options. While switching to opentofu/svchost would solve the logging issue, I have concerns about using it

  1. The README explicitly states their API is "experimental" with possible breaking changes
  2. No guarantee of maintaining compatibility with Terraform's service discovery protocol

The service discovery feature itself is valuable, but we need a more stable solution for the logging issue.

Options to consider:

  • Submit a PR to hashicorp/terraform-svchost to make logging configurable
  • Find an alternative way to suppress the debug output

@gringolito
Copy link
Copy Markdown
Author

I did some digging and verified that Terraform handles log redirection by setting its writer at initialization time. This means it delegates the responsibility of suppressing or redirecting CLI logs to the application rather than the libraries. As a result, the Terraform-provided libraries don’t try to control or silence log output, they assume the consuming application will handle it.

You can see this behavior in the logging initialization code here:
https://github.com/hashicorp/terraform/blob/e8631898c99cbc200e4088d7d748cfeb9799c161/internal/logging/logging.go#L55

Adding log output directly within the library may not align with what current consumers are expecting. That said, it might make more sense for the consuming applications to configure their own logging infrastructure instead of relying on, or assuming, that all libraries will enforce the same logging behavior by default.

@fujiwara
Copy link
Copy Markdown
Owner

fujiwara commented Dec 5, 2025

@gringolito

That said, it might make more sense for the consuming applications to configure their own logging infrastructure instead of relying on, or assuming, that all libraries will enforce the same logging behavior by default.

I see. But, all products using tfstate-lookup as a library will be surprised when it update to a new version.

I'll plan to workaround to hide debug logs. Thank you!

@fujiwara
Copy link
Copy Markdown
Owner

fujiwara commented Dec 5, 2025

@gringolito By the way, #254

But I don't have other environment like Terraform Enterprise or JFrog's Artifactory.
Could you please show me the test results on these environments if you can?

How about this?

@fujiwara fujiwara mentioned this pull request Dec 5, 2025
@fujiwara
Copy link
Copy Markdown
Owner

fujiwara commented Dec 5, 2025

I' working on pr254 7b47e2b

  • Because tfstate-lookup is a library, cannot do log.SetOutput as terraform.
    • The application's output must not be modified.
  • log.Println() uses log.std instance. So os.Stderr is already captured in it. We cannot replace after inited.

So, I have to no choice but to do a dirty hack like syscall.Dup(int(os.Stderr.Fd())).

Conclusion, This is problematic code as a library.

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.

2 participants