WIP - feat(dependency): implement outputs fetching from gcs #3327
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hello, |
|
It would be helpful to add integration tests for these changes. This will allow us to track and verify that the functionality works end-to-end in the future. |
TPXP
force-pushed
the
feat-gcs-remote-state-fetch
branch
from
cf09d17
to
4f9cac9
Compare
|
Hello, thanks a lot for the prompt feedback 🙌 I forgot to run pre-commit indeed, my bad. I ran it again, it looks better now. Regarding testing, I'm not sure how to proceed. I couldn't find tests for |
|
TPXP
force-pushed
the
feat-gcs-remote-state-fetch
branch
from
4f9cac9
to
e08bbb4
Compare
|
|
Hello, thanks for getting back to me 😃 I looked into the CI and its tests a little more and ran goimports, golint as well as a few tests. Hopefully all is good now 🤞 |
|
Hey @TPXP , It looks like this PR needs to be rebased off master. Please mark the PR as ready for review when it is. |
yhakbar
changed the title
TPXP
force-pushed
the
feat-gcs-remote-state-fetch
branch
from
e08bbb4
to
979d091
Compare
|
Hello, thanks for getting back to me. I've rebased the PR on the latest changes, let me know if there's anything else I can do to help :) |
TPXP
force-pushed
the
feat-gcs-remote-state-fetch
branch
from
979d091
to
6c8fb9b
Compare
TPXP
changed the title
|
Hello, |
TPXP
force-pushed
the
feat-gcs-remote-state-fetch
branch
from
6c8fb9b
to
eb63fc5
Compare
|
Hey @denis256 , thanks for your reply. I tried to adapt the AWS tests using this feature and added them to this PR. I can't say for sure if they work as I couldn't get them to run on my machine unfortunately (it seems the CI uses a magic |
|
Looks like integration_gcp_test.go need formatting |
TPXP
force-pushed
the
feat-gcs-remote-state-fetch
branch
2 times, most recently
from
d60e533
to
1617f5c
Compare
|
Hello @yhakbar, apologies for the delay. I took some time to address @denis256's feedback and fixed the merge conflicts.
Feel free to edit my branch to speed up review 🙏 |
TPXP
force-pushed
the
feat-gcs-remote-state-fetch
branch
from
1617f5c
to
8368349
Compare
yhakbar
requested changes
Sep 18, 2024
TPXP
force-pushed
the
feat-gcs-remote-state-fetch
branch
from
8368349
to
96064bc
Compare
|
Thanks for the testing @yhakbar , I've updated the code with your suggestion 👍 |
denis256
reviewed
Sep 20, 2024
| @@ -130,6 +133,80 @@ func TestGcpParallelStateInit(t *testing.T) { | |||
| runTerragrunt(t, "terragrunt run-all apply -auto-approve --terragrunt-non-interactive --terragrunt-working-dir "+tmpEnvPath) | |||
| } | |||
|
|
|||
| func TestGcpOutputFromRemoteState(t *testing.T) { //nolint: paralleltest | |||
There was a problem hiding this comment.
=== RUN TestGcpOutputFromRemoteState
integration_gcp_test.go:146: Copying fixtures/gcs-output-from-remote-state to /tmp/terragrunt-test3565337247
integration_gcp_test.go:149: Error writing temp file to /tmp/terragrunt-test3565337247: open /tmp/terragrunt-test3565337247: is a directory
panic.go:626: Deleting test GCS bucket terragrunt-test-bucket-yzmacx
panic.go:626: Failed to list objects and versions in GCS bucket terragrunt-test-bucket-yzmacx: storage: bucket doesn't exist
--- FAIL: TestGcpOutputFromRemoteState (0.14s)
There was a problem hiding this comment.
Thanks for running the tests, something was wrong indeed. I fixed this line, hopefully the test should pass now 🤞
There was a problem hiding this comment.
well... it still fails
=== RUN TestGcpMockOutputsFromRemoteState
integration_gcp_test.go:189: Copying fixtures/gcs-output-from-remote-state to /tmp/terragrunt-test3189224221
integration_gcp_test.go:197: [terragrunt apply --terragrunt-fetch-dependency-output-from-state --auto-approve --terragrunt-non-interactive --terragrunt-working-dir /tmp/terragrunt-test3189224221/fixtures/gcs-output-from-remote-state/env1/app1 --terragrunt-disable-log-formatting]
time=2024-09-20T17:51:09Z level=error prefix=. msg=Error: Unsupported block type
time=2024-09-20T17:51:09Z level=error prefix=. msg= on ../../terragrunt.hcl line 4, in remote_state:
time=2024-09-20T17:51:09Z level=error prefix=. msg= 4: generate {
time=2024-09-20T17:51:09Z level=error prefix=. msg=
time=2024-09-20T17:51:09Z level=error prefix=. msg=Blocks of type "generate" are not expected here. Did you mean to define argument
"generate"? If so, use the equals sign to assign it a value.
integration_gcp_test.go:197: Failed to run Terragrunt command 'terragrunt apply --terragrunt-fetch-dependency-output-from-state --auto-approve --terragrunt-non-interactive --terragrunt-working-dir /tmp/terragrunt-test3189224221/fixtures/gcs-output-from-remote-state/env1/app1' due to error: hcl.Diagnostics /tmp/terragrunt-test3189224221/fixtures/gcs-output-from-remote-state/terragrunt.hcl:4,3-11: Unsupported block type; Blocks of type "generate" are not expected here. Did you mean to define argument "generate"? If so, use the equals sign to assign it a value.
/home/circleci/project/config/hclparse/file.go:70 (0x10077ac)
(*File).Decode: return errors.WithStackTrace(err)
/home/circleci/project/config/config.go:937 (0x1609b3f)
decodeAsTerragruntConfigFile: if err := file.Decode(&terragruntConfig, evalContext); err != nil {
/home/circleci/project/config/config.go:864 (0x16092af)
ParseConfig: terragruntConfigFile, err := decodeAsTerragruntConfigFile(ctx, file, evalContext)
/home/circleci/project/config/config.go:773 (0x1608eb8)
ParseConfigFile.func1: config, err = ParseConfig(ctx, file, includeFromChild) //nolint:contextcheck
/home/circleci/project/telemetry/metrics.go:42 (0x10a0793)
Time: return fn(ctx)
/home/circleci/project/telemetry/telemetry.go:80 (0x1608aad)
ParseConfigFile.Telemetry.func2: return Time(ctx, name, attrs, fn)
/home/circleci/project/telemetry/traces.go:38 (0x10a30dd)
Trace: return fn(ctx)
/home/circleci/project/telemetry/telemetry.go:79 (0x1608a25)
ParseConfigFile: return Trace(ctx, name, attrs, func(ctx context.Context) error {
/home/circleci/project/config/config.go:730 (0x16087f3)
ParseConfigFile: err := telemetry.Telemetry(ctx, ctx.TerragruntOptions, "parse_config_file", map[string]interface{}{
/home/circleci/project/config/include.go:96 (0x16276f7)
parseIncludedConfig: return ParseConfigFile(ctx, includePath, includedConfig)
/home/circleci/project/config/include.go:128 (0x162790a)
handleInclude: parsedIncludeConfig, err = parseIncludedConfig(ctx, &includeConfig)
/home/circleci/project/config/config.go:880 (0x1609310)
ParseConfig: mergedConfig, err := handleInclude(ctx, config, false)
/home/circleci/project/config/config.go:773 (0x1608eb8)
ParseConfigFile.func1: config, err = ParseConfig(ctx, file, includeFromChild) //nolint:contextcheck
/home/circleci/project/telemetry/metrics.go:42 (0x10a0793)
Time: return fn(ctx)
/home/circleci/project/telemetry/telemetry.go:80 (0x1608aad)
ParseConfigFile.Telemetry.func2: return Time(ctx, name, attrs, fn)
/home/circleci/project/telemetry/traces.go:38 (0x10a30dd)
Trace: return fn(ctx)
/home/circleci/project/telemetry/telemetry.go:79 (0x1608a25)
ParseConfigFile: return Trace(ctx, name, attrs, func(ctx context.Context) error {
/home/circleci/project/config/config.go:730 (0x16087f3)
ParseConfigFile: err := telemetry.Telemetry(ctx, ctx.TerragruntOptions, "parse_config_file", map[string]interface{}{
/home/circleci/project/config/config.go:720 (0x160871b)
ReadTerragruntConfig: return ParseConfigFile(parcingCtx, terragruntOptions.TerragruntConfigPath, nil) //nolint:contextcheck
/home/circleci/project/cli/commands/terraform/action.go:101 (0x184dcc5)
runTerraform: terragruntConfig, err := config.ReadTerragruntConfig(ctx, terragruntOptions, config.DefaultParserOptions(terragruntOptions))
/home/circleci/project/cli/commands/terraform/action.go:83 (0x184dabc)
Run: return runTerraform(ctx, opts, new(Target))
/home/circleci/project/cli/commands/terraform/command.go:47 (0x1e48a4d)
NewApp.NewCommand.Action.func5: return Run(ctx.Context, opts.OptionsFromContext(ctx))
/home/circleci/project/cli/app.go:239 (0x1e4a647)
runAction.func2: return action(cliCtx)
/home/circleci/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:78 (0x1603036)
(*Group).Go.func1: if err := f(); err != nil {
/usr/local/go/src/runtime/asm_amd64.s:1695 (0x47aae1)
goexit: BYTE $0x90 // NOP
Stdout: (see log output above)
Stderr: (see log output above)
panic.go:626: Deleting test GCS bucket terragrunt-test-bucket-3f2cvv
panic.go:626: Failed to list objects and versions in GCS bucket terragrunt-test-bucket-3f2cvv: storage: bucket doesn't exist
--- FAIL: TestGcpMockOutputsFromRemoteState (0.22s)
There was a problem hiding this comment.
Updated the test again, it should be OK now
TPXP
force-pushed
the
feat-gcs-remote-state-fetch
branch
2 times, most recently
from
d230818
to
b7681ad
Compare
This implements outputs fetching from the GCS backend, in a similar fashion to what has been implemented for S3. I've also added some misc fixes: * correct typo: steateBody -> stateBody * drop unused Path parameter on GCS config (it's not supported) Tested on my architecture, everything seems to work! Ref: https://developer.hashicorp.com/terraform/language/settings/backends/gcs
TPXP
force-pushed
the
feat-gcs-remote-state-fetch
branch
from
b7681ad
to
04e854c
Compare
Looks like it still fails |
denis256
reviewed
Oct 1, 2024
| terragruntOptions *options.TerragruntOptions, | ||
| remoteState *remote.RemoteState, | ||
| ) ([]byte, error) { | ||
| terragruntOptions.Logger.Debugf("Fetching outputs directly from gcs://%s/%s/default.tfstate", remoteState.Config["bucket"], remoteState.Config["prefix"]) |
There was a problem hiding this comment.
Why not use remoteState.Config["key"] instead of hardcoded default.tfstate" ?
| bucket := gcsClient.Bucket(gcsConfig.Bucket) | ||
| object := bucket.Object(gcsConfig.Prefix + "/default.tfstate") | ||
|
|
||
| reader, err := object.NewReader(context.Background()) |
There was a problem hiding this comment.
I was wondering if can be propagated current context
|
Hey @TPXP , This PR has gone stale again, so I'm going to move it to draft, and mark it WIP. When you've done the following, please mark the PR as ready for review again:
|
yhakbar
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This implements outputs fetching from the GCS backend, in a similar fashion to what has been implemented for S3.
I've also added some misc fixes:
steateBody->stateBodyTested on my architecture, everything seems to work!
TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Added support for the GCS backend for dependency output fetching (
--terragrunt-fetch-dependency-output-from-state)Migration Guide
None, this makes this error message go away (and speeds up dependency output state resolution)