backend/remote: change how to fall back to a local backend #19378
Conversation
|
I am currently working to fix the tests related to backends in the |
svanharmelen
force-pushed
the
svh/f-remote-state
branch
from
0c259fb to
b91a14d
Compare
svanharmelen
changed the title
svanharmelen
force-pushed
the
svh/f-remote-state
branch
2 times, most recently
from
9421391 to
3b1677f
Compare
There was a problem hiding this comment.
Looks reasonable to me as an incremental improvement.
The fact that we now not only have the local backend wrapping other backends but also the remote backend wrapping the local backend just affirms for me that we've not got these interfaces right yet and will want to do some deeper rework of the design here in a later release, but this will get us where we need to go for now.
backend/remote/backend.go
Outdated
| if err == tfe.ErrResourceNotFound { | ||
| err = fmt.Errorf("organization %s does not exist", b.organization) | ||
| } | ||
| diags = diags.Append(tfdiags.Sourceless( |
There was a problem hiding this comment.
It seems like errors here are essentially validation for the organization argument, so perhaps better to return a tfdiags.AttributeValue result here instead so that the error can point directly to that argument, rather than to the block as a whole?
I can imagine this returning errors that are not strictly user config errors, like the TFE API being totally unreachable, but I think the detail message below makes it clear that we were making a call out here so the resulting full rendered error message should still be clear.
backend/remote/backend.go
Outdated
| // Check if the workspace allows remote operations or that | ||
| // TF_FORCE_LOCAL_BACKEND is set. In both cases we will use | ||
| // the local backend to run the operation. | ||
| if !w.Operations || os.Getenv("TF_FORCE_LOCAL_BACKEND") != "" { |
There was a problem hiding this comment.
Could we instead handle this environment variable inside the Configure function and set a flag on the backend object from it?
I think it's clearer to handle inputs from all external sources in one place and then have the rest of these functions just access settings from the b object, since then it's easier to quickly understand from Configure what different inputs will affect the behavior of this backend.
backend/remote/backend_apply_test.go
Outdated
| if err := os.Setenv("TF_FORCE_LOCAL_BACKEND", "1"); err != nil { | ||
| t.Fatalf("error setting environment variable TF_FORCE_LOCAL_BACKEND: %v", err) | ||
| } | ||
| defer os.Unsetenv("TF_FORCE_LOCAL_BACKEND") |
There was a problem hiding this comment.
Perhaps we should also force this unset in TestMain so that having it set ambiently won't affect the test outcome? Seems unlikely in general use, but I could imagine accidentally leaving it set after some testing and then finding that the tests behave differently and being confused as to why.
There was a problem hiding this comment.
Sure, will add that before merging.
In order to support free organizations, we need a way to load the `remote` backend and then, depending on the used offering/plan, enable or disable remote operations. In other words, we should be able to dynamically fall back to the `local` backend if needed, after first configuring the `remote` backend. To make this works we need to change the way this was done previously when the env var `TF_FORCE_LOCAL_BACKEND` was set. The clear difference of course being that the env var would be available on startup, while the used offering/plan is only known after being able to connect to TFE.
svanharmelen
force-pushed
the
svh/f-remote-state
branch
from
3b1677f to
a17f317
Compare
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
In order to support free organizations, we need a way to load the
remotebackend and then, depending on the used offering/plan, enable or disable remote operations.In other words, we should be able to dynamically fall back to the
localbackend if needed, after first configuring theremotebackend.To make this work we need to change the way this was done previously when the env var
TF_FORCE_LOCAL_BACKENDwas set. The clear difference of course being that the env var would be available on startup, while the used offering/plan is only known after being able to connect to TFE.