Azurerm backend timeout (#2263)

adds a timeout_seconds configuration variable with a default value to the AzureRM backend
Signed-off-by: Ilia Gogotchuri <ilia.gogotchuri0@gmail.com>
This commit is contained in:
Ilia Gogotchuri 2024-12-06 17:32:23 +04:00 committed by GitHub
parent 85dc2615ad
commit ffa43acfcd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 199 additions and 55 deletions

View File

@ -45,6 +45,7 @@ ENHANCEMENTS:
* `tofu test` now supports `override_resource` and `override_data` blocks in the scope of a single `mock_provider`. ([#2168](https://github.com/opentofu/opentofu/pull/2168))
* Changes to encryption configuration now auto-apply the migration ([#2232](https://github.com/opentofu/opentofu/pull/2232))
* References to vars, data, etc. are now usable in variable validation ([#2216](https://github.com/opentofu/opentofu/pull/2216))
* `AzureRM` backend now support `timeout_seconds` with default timeout of 300 seconds ([#2263](https://github.com/opentofu/opentofu/pull/2263))
BUG FIXES:
* `templatefile` no longer crashes if the given filename is derived from a sensitive value. ([#1801](https://github.com/opentofu/opentofu/issues/1801))

View File

@ -40,6 +40,7 @@ type ArmClient struct {
resourceGroupName string
storageAccountName string
sasToken string
timeoutSeconds int
}
func buildArmClient(ctx context.Context, config BackendConfig) (*ArmClient, error) {
@ -52,6 +53,7 @@ func buildArmClient(ctx context.Context, config BackendConfig) (*ArmClient, erro
environment: *env,
resourceGroupName: config.ResourceGroupName,
storageAccountName: config.StorageAccountName,
timeoutSeconds: config.TimeoutSeconds,
}
// if we have an Access Key - we don't need the other clients
@ -163,7 +165,9 @@ func (c ArmClient) getBlobClient(ctx context.Context) (*blobs.Client, error) {
accessKey := c.accessKey
if accessKey == "" {
log.Printf("[DEBUG] Building the Blob Client from an Access Token (using user credentials)")
keys, err := c.storageAccountsClient.ListKeys(ctx, c.resourceGroupName, c.storageAccountName, "")
timeoutCtx, cancel := context.WithTimeout(ctx, time.Duration(c.timeoutSeconds)*time.Second)
defer cancel()
keys, err := c.storageAccountsClient.ListKeys(timeoutCtx, c.resourceGroupName, c.storageAccountName, "")
if err != nil {
return nil, fmt.Errorf("Error retrieving keys for Storage Account %q: %w", c.storageAccountName, err)
}
@ -208,7 +212,9 @@ func (c ArmClient) getContainersClient(ctx context.Context) (*containers.Client,
accessKey := c.accessKey
if accessKey == "" {
log.Printf("[DEBUG] Building the Container Client from an Access Token (using user credentials)")
keys, err := c.storageAccountsClient.ListKeys(ctx, c.resourceGroupName, c.storageAccountName, "")
timeoutCtx, cancel := context.WithTimeout(ctx, time.Duration(c.timeoutSeconds)*time.Second)
defer cancel()
keys, err := c.storageAccountsClient.ListKeys(timeoutCtx, c.resourceGroupName, c.storageAccountName, "")
if err != nil {
return nil, fmt.Errorf("Error retrieving keys for Storage Account %q: %w", c.storageAccountName, err)
}

View File

@ -14,6 +14,8 @@ import (
"github.com/opentofu/opentofu/internal/legacy/helper/schema"
)
const defaultTimeout = 300 // 5 minutes
// New creates a new backend for Azure remote state.
func New(enc encryption.StateEncryption) backend.Backend {
s := &schema.Backend{
@ -91,6 +93,20 @@ func New(enc encryption.StateEncryption) backend.Backend {
DefaultFunc: schema.EnvDefaultFunc("ARM_ENDPOINT", ""),
},
"timeout_seconds": {
Type: schema.TypeInt,
Optional: true,
Description: "The timeout in seconds for initializing a client or retrieving a Blob or a Metadata from Azure.",
DefaultFunc: schema.EnvDefaultFunc("ARM_TIMEOUT_SECONDS", defaultTimeout),
ValidateFunc: func(v interface{}, _ string) ([]string, []error) {
value, ok := v.(int)
if !ok || value < 0 {
return nil, []error{fmt.Errorf("timeout_seconds expected to be a non-negative integer")}
}
return nil, nil
},
},
"subscription_id": {
Type: schema.TypeString,
Optional: true,
@ -211,6 +227,7 @@ type BackendConfig struct {
ClientCertificatePath string
ClientSecret string
CustomResourceManagerEndpoint string
TimeoutSeconds int
MetadataHost string
Environment string
MsiEndpoint string
@ -227,6 +244,7 @@ type BackendConfig struct {
UseAzureADAuthentication bool
}
//nolint:errcheck //at this stage type conversion is safe
func (b *Backend) configure(ctx context.Context) error {
if b.containerName != "" {
return nil
@ -246,6 +264,7 @@ func (b *Backend) configure(ctx context.Context) error {
ClientCertificatePath: data.Get("client_certificate_path").(string),
ClientSecret: data.Get("client_secret").(string),
CustomResourceManagerEndpoint: data.Get("endpoint").(string),
TimeoutSeconds: data.Get("timeout_seconds").(int),
MetadataHost: data.Get("metadata_host").(string),
Environment: data.Get("environment").(string),
MsiEndpoint: data.Get("msi_endpoint").(string),

View File

@ -96,6 +96,7 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) {
keyName: b.path(name),
accountName: b.accountName,
snapshot: b.snapshot,
timeoutSeconds: b.armClient.timeoutSeconds,
}
stateMgr := remote.NewState(client, b.encryption)

View File

@ -45,6 +45,63 @@ func TestBackendConfig(t *testing.T) {
}
}
func TestBackendConfig_Timeout(t *testing.T) {
config := map[string]any{
"storage_account_name": "tfaccount",
"container_name": "tfcontainer",
"key": "state",
"snapshot": false,
// Access Key must be Base64
"access_key": "QUNDRVNTX0tFWQ0K",
}
testCases := []struct {
name string
timeoutSeconds any
expectError bool
}{
{
name: "string timeout",
timeoutSeconds: "Nonsense",
expectError: true,
},
{
name: "negative timeout",
timeoutSeconds: -10,
expectError: true,
},
{
// 0 is a valid timeout value, it disables the timeout
name: "zero timeout",
timeoutSeconds: 0,
},
{
name: "positive timeout",
timeoutSeconds: 10,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
config["timeout_seconds"] = tc.timeoutSeconds
b, _, errors := backend.TestBackendConfigWarningsAndErrors(t, New(encryption.StateEncryptionDisabled()), backend.TestWrapConfig(config))
if tc.expectError {
if len(errors) == 0 {
t.Fatalf("Expected an error")
}
return
}
if !tc.expectError && len(errors) > 0 {
t.Fatalf("Expected no errors, got: %v", errors)
}
be, ok := b.(*Backend)
if !ok || be == nil {
t.Fatalf("Expected initialized Backend, got %T", b)
}
if be.armClient.timeoutSeconds != tc.timeoutSeconds {
t.Fatalf("Expected timeoutSeconds to be %d, got %d", tc.timeoutSeconds, be.armClient.timeoutSeconds)
}
})
}
}
func TestAccBackendAccessKeyBasic(t *testing.T) {
testAccAzureBackend(t)
rs := acctest.RandString(4)

View File

@ -12,6 +12,7 @@ import (
"fmt"
"log"
"net/http"
"time"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/go-uuid"
@ -31,18 +32,16 @@ type RemoteClient struct {
accountName string
containerName string
keyName string
leaseID string
leaseID *string
snapshot bool
timeoutSeconds int
}
func (c *RemoteClient) Get() (*remote.Payload, error) {
options := blobs.GetInput{}
if c.leaseID != "" {
options.LeaseID = &c.leaseID
}
ctx := context.TODO()
blob, err := c.giovanniBlobClient.Get(ctx, c.accountName, c.containerName, c.keyName, options)
// Get should time out after the timeoutSeconds
ctx, ctxCancel := c.getContextWithTimeout()
defer ctxCancel()
blob, err := c.giovanniBlobClient.Get(ctx, c.accountName, c.containerName, c.keyName, blobs.GetInput{LeaseID: c.leaseID})
if err != nil {
if blob.Response.IsHTTPStatus(http.StatusNotFound) {
return nil, nil
@ -63,23 +62,9 @@ func (c *RemoteClient) Get() (*remote.Payload, error) {
}
func (c *RemoteClient) Put(data []byte) error {
getOptions := blobs.GetPropertiesInput{}
setOptions := blobs.SetPropertiesInput{}
putOptions := blobs.PutBlockBlobInput{}
options := blobs.GetInput{}
if c.leaseID != "" {
options.LeaseID = &c.leaseID
getOptions.LeaseID = &c.leaseID
setOptions.LeaseID = &c.leaseID
putOptions.LeaseID = &c.leaseID
}
ctx := context.TODO()
if c.snapshot {
snapshotInput := blobs.SnapshotInput{LeaseID: options.LeaseID}
snapshotInput := blobs.SnapshotInput{LeaseID: c.leaseID}
log.Printf("[DEBUG] Snapshotting existing Blob %q (Container %q / Account %q)", c.keyName, c.containerName, c.accountName)
if _, err := c.giovanniBlobClient.Snapshot(ctx, c.accountName, c.containerName, c.keyName, snapshotInput); err != nil {
return fmt.Errorf("error snapshotting Blob %q (Container %q / Account %q): %w", c.keyName, c.containerName, c.accountName, err)
@ -88,31 +73,28 @@ func (c *RemoteClient) Put(data []byte) error {
log.Print("[DEBUG] Created blob snapshot")
}
blob, err := c.giovanniBlobClient.GetProperties(ctx, c.accountName, c.containerName, c.keyName, getOptions)
properties, err := c.getBlobProperties()
if err != nil {
if blob.StatusCode != 404 {
if properties.StatusCode != http.StatusNotFound {
return err
}
}
contentType := "application/json"
putOptions.Content = &data
putOptions.ContentType = &contentType
putOptions.MetaData = blob.MetaData
putOptions := blobs.PutBlockBlobInput{
LeaseID: c.leaseID,
Content: &data,
ContentType: &contentType,
MetaData: properties.MetaData,
}
_, err = c.giovanniBlobClient.PutBlockBlob(ctx, c.accountName, c.containerName, c.keyName, putOptions)
return err
}
func (c *RemoteClient) Delete() error {
options := blobs.DeleteInput{}
if c.leaseID != "" {
options.LeaseID = &c.leaseID
}
ctx := context.TODO()
resp, err := c.giovanniBlobClient.Delete(ctx, c.accountName, c.containerName, c.keyName, options)
resp, err := c.giovanniBlobClient.Delete(ctx, c.accountName, c.containerName, c.keyName, blobs.DeleteInput{LeaseID: c.leaseID})
if err != nil {
if !resp.IsHTTPStatus(http.StatusNotFound) {
return err
@ -153,14 +135,13 @@ func (c *RemoteClient) Lock(info *statemgr.LockInfo) (string, error) {
ctx := context.TODO()
// obtain properties to see if the blob lease is already in use. If the blob doesn't exist, create it
properties, err := c.giovanniBlobClient.GetProperties(ctx, c.accountName, c.containerName, c.keyName, blobs.GetPropertiesInput{})
properties, err := c.getBlobProperties()
if err != nil {
// error if we had issues getting the blob
if !properties.Response.IsHTTPStatus(http.StatusNotFound) {
return "", getLockInfoErr(err)
}
// if we don't find the blob, we need to build it
contentType := "application/json"
putGOptions := blobs.PutBlockBlobInput{
ContentType: &contentType,
@ -183,7 +164,7 @@ func (c *RemoteClient) Lock(info *statemgr.LockInfo) (string, error) {
}
info.ID = leaseID.LeaseID
c.leaseID = leaseID.LeaseID
c.setLeaseID(leaseID.LeaseID)
if err := c.writeLockInfo(info); err != nil {
return "", err
@ -193,18 +174,12 @@ func (c *RemoteClient) Lock(info *statemgr.LockInfo) (string, error) {
}
func (c *RemoteClient) getLockInfo() (*statemgr.LockInfo, error) {
options := blobs.GetPropertiesInput{}
if c.leaseID != "" {
options.LeaseID = &c.leaseID
}
ctx := context.TODO()
blob, err := c.giovanniBlobClient.GetProperties(ctx, c.accountName, c.containerName, c.keyName, options)
properties, err := c.getBlobProperties()
if err != nil {
return nil, err
}
raw := blob.MetaData[lockInfoMetaKey]
raw := properties.MetaData[lockInfoMetaKey]
if raw == "" {
return nil, fmt.Errorf("blob metadata %q was empty", lockInfoMetaKey)
}
@ -226,21 +201,21 @@ func (c *RemoteClient) getLockInfo() (*statemgr.LockInfo, error) {
// writes info to blob meta data, deletes metadata entry if info is nil
func (c *RemoteClient) writeLockInfo(info *statemgr.LockInfo) error {
ctx := context.TODO()
blob, err := c.giovanniBlobClient.GetProperties(ctx, c.accountName, c.containerName, c.keyName, blobs.GetPropertiesInput{LeaseID: &c.leaseID})
properties, err := c.getBlobProperties()
if err != nil {
return err
}
if info == nil {
delete(blob.MetaData, lockInfoMetaKey)
delete(properties.MetaData, lockInfoMetaKey)
} else {
value := base64.StdEncoding.EncodeToString(info.Marshal())
blob.MetaData[lockInfoMetaKey] = value
properties.MetaData[lockInfoMetaKey] = value
}
opts := blobs.SetMetaDataInput{
LeaseID: &c.leaseID,
MetaData: blob.MetaData,
LeaseID: c.leaseID,
MetaData: properties.MetaData,
}
_, err = c.giovanniBlobClient.SetMetaData(ctx, c.accountName, c.containerName, c.keyName, opts)
@ -262,7 +237,7 @@ func (c *RemoteClient) Unlock(id string) error {
return lockErr
}
c.leaseID = lockInfo.ID
c.setLeaseID(lockInfo.ID)
if err := c.writeLockInfo(nil); err != nil {
lockErr.Err = fmt.Errorf("failed to delete lock info from metadata: %w", err)
return lockErr
@ -275,7 +250,29 @@ func (c *RemoteClient) Unlock(id string) error {
return lockErr
}
c.leaseID = ""
c.leaseID = nil
return nil
}
// getBlobProperties wraps the GetProperties method of the giovanniBlobClient with timeout
func (c *RemoteClient) getBlobProperties() (blobs.GetPropertiesResult, error) {
ctx, ctxCancel := c.getContextWithTimeout()
defer ctxCancel()
return c.giovanniBlobClient.GetProperties(ctx, c.accountName, c.containerName, c.keyName, blobs.GetPropertiesInput{LeaseID: c.leaseID})
}
// getContextWithTimeout returns a context with timeout based on the timeoutSeconds
func (c *RemoteClient) getContextWithTimeout() (context.Context, context.CancelFunc) {
return context.WithTimeout(context.Background(), time.Duration(c.timeoutSeconds)*time.Second)
}
// setLeaseID takes a string leaseID and sets the leaseID field of the RemoteClient
// if passed leaseID is empty, it sets the leaseID field to nil
func (c *RemoteClient) setLeaseID(leaseID string) {
if leaseID == "" {
c.leaseID = nil
} else {
c.leaseID = &leaseID
}
}

View File

@ -6,6 +6,7 @@
package backend
import (
"fmt"
"reflect"
"sort"
"testing"
@ -22,6 +23,62 @@ import (
"github.com/opentofu/opentofu/internal/tfdiags"
)
func separateWarningsAndErrors(diags tfdiags.Diagnostics) ([]string, []error) {
warnings := make([]string, 0)
errors := make([]error, 0)
for _, diag := range diags {
if diag.Severity() == tfdiags.Warning {
warnings = append(warnings, diag.Description().Summary)
} else if diag.Severity() == tfdiags.Error {
errors = append(errors, fmt.Errorf("%s", diag.Description().Summary))
}
}
return warnings, errors
}
// TestBackendConfigWarningsAndErrors validates and configures the backend with the
// given configuration and returns backend and any warnings and errors encountered.
// used to test validations
func TestBackendConfigWarningsAndErrors(t *testing.T, b Backend, c hcl.Body) (Backend, []string, []error) {
t.Helper()
t.Logf("TestBackendConfig on %T with %#v", b, c)
var diags tfdiags.Diagnostics
// To make things easier for test authors, we'll allow a nil body here
// (even though that's not normally valid) and just treat it as an empty
// body.
if c == nil {
c = hcl.EmptyBody()
}
schema := b.ConfigSchema()
spec := schema.DecoderSpec()
obj, decDiags := hcldec.Decode(c, spec, nil)
diags = diags.Append(decDiags)
newObj, valDiags := b.PrepareConfig(obj)
diags = diags.Append(valDiags.InConfigBody(c, ""))
// it's valid for a Backend to have warnings (e.g. a Deprecation) as such we should only raise on errors
if len(diags) != 0 {
warnings, errors := separateWarningsAndErrors(diags)
return nil, warnings, errors
}
obj = newObj
confDiags := b.Configure(obj)
if len(confDiags) != 0 {
confDiags = confDiags.InConfigBody(c, "")
warnings, errors := separateWarningsAndErrors(confDiags)
return nil, warnings, errors
}
return b, nil, nil
}
// TestBackendConfig validates and configures the backend with the
// given configuration.
func TestBackendConfig(t *testing.T, b Backend, c hcl.Body) Backend {

View File

@ -256,6 +256,12 @@ The following configuration options are supported:
An `endpoint` should only be configured when using Azure Stack.
:::
* `timeout_seconds` - (Optional) The number of seconds before a timeout is reached when attempting to initialize a client, retrieve a Blob or a Metadata from Azure. This can also be sourced from the `ARM_TIMEOUT_SECONDS` environment variable. Defaults to `300` (5 minutes). To disable the timeout, set this to `0`.
:::warning Note
Setting `timeout_seconds` to `0` or a large value only disables/extends timeouts originating from OpenTofu. Requests will still time out based on your system's network configuration.
:::
* `metadata_host` - (Optional) The Hostname of the Azure Metadata Service (for example `management.azure.com`), used to obtain the Cloud Environment when using a Custom Azure Environment. This can also be sourced from the `ARM_METADATA_HOSTNAME` Environment Variable.
* `snapshot` - (Optional) Should the Blob used to store the OpenTofu Statefile be snapshotted before use? Defaults to `false`. This value can also be sourced from the `ARM_SNAPSHOT` environment variable.