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

Initial experimental support for deferred actions when count or for_each are unknown #34651

Merged
merged 13 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions internal/addrs/partial_expanded.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,34 @@ func (per PartialExpandedResource) KnownModuleInstancePrefix() ModuleInstance {
return per.module.KnownPrefix()
}

// ModuleInstance returns the fully-qualified [ModuleInstance] that this
// partial-expanded resource belongs to, but only if its module instance
// address is fully known.
//
// The second return value is false if the module instance address is not
// fully expanded, in which case the first return value is invalid. Use
// [PartialExpandedResource.PartialExpandedModule] instead in that case.
func (per PartialExpandedResource) ModuleInstance() (ModuleInstance, bool) {
if len(per.module.unexpandedSuffix) != 0 {
return nil, false
}
return per.module.expandedPrefix, true
}

// PartialExpandedModule returns a [PartialExpandedModule] address describing
// the partially-unknown module instance address that the resource belongs to,
// but only if the module instance address is not fully known.
//
// The second return value is false if the module instance address is actually
// fully expanded, in which case the first return value is invalid. Use
// [PartialExpandedResource.ModuleInstance] instead in that case.
func (per PartialExpandedResource) PartialExpandedModule() (PartialExpandedModule, bool) {
if len(per.module.unexpandedSuffix) == 0 {
return PartialExpandedModule{}, false
}
return per.module, true
}

// String returns a string representation of the pattern which uses the special
// placeholder "[*]" to represent positions where instance keys are not yet
// known.
Expand Down
7 changes: 5 additions & 2 deletions internal/instances/expander.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,9 @@ func (e *Expander) ExpandModule(addr addrs.Module) []addrs.ModuleInstance {
// key type: integer keys are sorted numerically, and string keys are sorted
// lexically.
func (e *Expander) ExpandAbsModuleCall(addr addrs.AbsModuleCall) (keyType addrs.InstanceKeyType, insts []addrs.InstanceKey, known bool) {
e.mu.RLock()
defer e.mu.RUnlock()

expParent, ok := e.findModule(addr.Module)
if !ok {
// This module call lives under an unknown-expansion prefix, so we
Expand Down Expand Up @@ -415,8 +418,8 @@ func (e *Expander) GetResourceInstanceRepetitionData(addr addrs.AbsResourceInsta
return exp.repetitionData(addr.Resource.Key)
}

// GetModuleCallInstanceKeys determines the child instance keys for one specific
// instance of a module call.
// ResourceInstanceKeys determines the child instance keys for one specific
// instance of a resource.
//
// keyType describes the expected type of all keys in knownKeys, which typically
// also implies what data type would be used to describe the full set of
Expand Down
14 changes: 13 additions & 1 deletion internal/instances/expansion_mode.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,19 @@ import (
// ways expansion can operate depending on how repetition is configured for
// an object.
type expansion interface {
instanceKeys() (addrs.InstanceKeyType, []addrs.InstanceKey, bool)
// instanceKeys determines the full set of instance keys for whatever
// object this expansion is associated with, or indicates that we
// can't yet know what they are (using keysUnknown=true).
//
// if keysUnknown is true then the "keys" result is likely to be incomplete,
// meaning that there might be other instance keys that we've not yet
// calculated, but we know that they will be of type keyType.
//
// If len(keys) == 0 when keysUnknown is true then we don't yet know _any_
// instance keys for this object. keyType might be [addrs.UnknownKeyType]
// if we don't even have enough information to predict what type of
// keys we will be using for this object.
instanceKeys() (keyType addrs.InstanceKeyType, keys []addrs.InstanceKey, keysUnknown bool)
repetitionData(addrs.InstanceKey) RepetitionData
}

Expand Down
322 changes: 322 additions & 0 deletions internal/plans/deferring/deferred.go

Large diffs are not rendered by default.

24 changes: 24 additions & 0 deletions internal/plans/deferring/deferred_partial_expanded_resource.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package deferring

import (
"github.com/zclconf/go-cty/cty"
)

// deferredPartialExpandedResource tracks placeholder information for an
// unbounded set of potential resource instances sharing a common known
// address prefix.
//
// This is for situations where we can't even predict which instances of
// a resource will be declared, due to a count or for_each argument being
// unknown. The unknown repetition argument could either be on the resource
// itself or on one of its ancestor module calls.
type deferredPartialExpandedResource struct {
// valuePlaceholder is a placeholder value describes what all of the
// potential instances in the unbounded set represented by this object
// have in common, using unknown values for any parts where we cannot
// guarantee that all instances will agree.
valuePlaceholder cty.Value
}
38 changes: 38 additions & 0 deletions internal/plans/deferring/deferred_resource_instance.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package deferring

import (
"github.com/hashicorp/terraform/internal/plans"
"github.com/zclconf/go-cty/cty"
)

// deferredResourceInstance tracks information about a resource instance whose
// address is precisely known but whose planned action has been deferred for
// some other reason.
type deferredResourceInstance struct {
// plannedAction is the action that Terraform expects to take for this
// resource instance in a future round.
//
// This can be set to plans.Undecided in situations where there isn't
// even enough information to decide what the action would be.
plannedAction plans.Action

// plannedValue is an approximation of the value that Terraform expects
// to plan for this resource instance in a future round, using unknown
// values in locations where a concrete value cannot yet be decided.
//
// In the most extreme case, plannedValue could be cty.DynamicVal to
// reflect that we know nothing at all about the resource instance, or
// an unknown value of the resource instance's schema type if the values
// are completely unknown but we've at least got enough information to
// approximate the type of the value.
//
// However, ideally this should be a known object value that potentially
// has unknown values for individual attributes inside, since that gives
// the most context to aid in finding errors that would definitely arise
// on a future round, and thus shorten the iteration time to find that
// problem.
plannedValue cty.Value
}
165 changes: 165 additions & 0 deletions internal/plans/deferring/deferred_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package deferring

import (
"testing"

"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/plans"
"github.com/zclconf/go-cty/cty"
)

func TestDeferred_externalDependency(t *testing.T) {
// The resource graph is irrelevant for this case, because we're going to
// defer any resource instance changes regardless. Therefore an empty
// graph is just fine.
resourceGraph := addrs.NewDirectedGraph[addrs.ConfigResource]()
deferred := NewDeferred(resourceGraph)

// This reports that something outside of the modules runtime knows that
// everything in this configuration depends on some elsewhere-action
// that has been deferred, and so the modules runtime must respect that
// even though it doesn't know the details of why it is so.
deferred.SetExternalDependencyDeferred()

// With the above flag set, now ShouldDeferResourceInstanceChanges should
// return true regardless of any other information.
got := deferred.ShouldDeferResourceInstanceChanges(addrs.AbsResourceInstance{
Module: addrs.RootModuleInstance,
Resource: addrs.ResourceInstance{
Resource: addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "anything",
Name: "really-anything",
},
},
})
if !got {
t.Errorf("did not report that the instance should have its changes deferred; should have")
}
}

func TestDeferred_absResourceInstanceDeferred(t *testing.T) {
instAAddr := addrs.AbsResourceInstance{
Module: addrs.RootModuleInstance.Child("foo", addrs.NoKey),
Resource: addrs.ResourceInstance{
Resource: addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "test",
Name: "a",
},
},
}
instBAddr := addrs.AbsResourceInstance{
Module: addrs.RootModuleInstance,
Resource: addrs.ResourceInstance{
Resource: addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "test",
Name: "a",
},
},
}
instCAddr := addrs.AbsResourceInstance{
Module: addrs.RootModuleInstance,
Resource: addrs.ResourceInstance{
Resource: addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "test",
Name: "c",
},
},
}

resourceGraph := addrs.NewDirectedGraph[addrs.ConfigResource]()
resourceGraph.AddDependency(instCAddr.ConfigResource(), instBAddr.ConfigResource())
resourceGraph.AddDependency(instCAddr.ConfigResource(), instAAddr.ConfigResource())
deferred := NewDeferred(resourceGraph)

// Before we report anything, all three addresses should indicate that
// they don't need to have their actions deferred.
t.Run("without any deferrals yet", func(t *testing.T) {
for _, instAddr := range []addrs.AbsResourceInstance{instAAddr, instBAddr, instCAddr} {
if deferred.ShouldDeferResourceInstanceChanges(instAddr) {
t.Errorf("%s reported as needing deferred; should not be, yet", instAddr)
}
}
})

// Instance A has its Create action deferred for some reason.
deferred.ReportResourceInstanceDeferred(instAAddr, plans.Create, cty.DynamicVal)

t.Run("with one resource instance deferred", func(t *testing.T) {
if !deferred.ShouldDeferResourceInstanceChanges(instCAddr) {
t.Errorf("%s was not reported as needing deferred; should be deferred", instCAddr)
}
if deferred.ShouldDeferResourceInstanceChanges(instBAddr) {
t.Errorf("%s reported as needing deferred; should not be", instCAddr)
}
})
}

func TestDeferred_partialExpandedResource(t *testing.T) {
instAAddr := addrs.AbsResourceInstance{
Module: addrs.RootModuleInstance.Child("foo", addrs.NoKey),
Resource: addrs.ResourceInstance{
Resource: addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "test",
Name: "a",
},
},
}
instBAddr := addrs.AbsResourceInstance{
Module: addrs.RootModuleInstance,
Resource: addrs.ResourceInstance{
Resource: addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "test",
Name: "a",
},
},
}
instCAddr := addrs.AbsResourceInstance{
Module: addrs.RootModuleInstance,
Resource: addrs.ResourceInstance{
Resource: addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "test",
Name: "c",
},
},
}
instAPartial := addrs.RootModuleInstance.
UnexpandedChild(addrs.ModuleCall{Name: "foo"}).
Resource(instAAddr.Resource.Resource)

resourceGraph := addrs.NewDirectedGraph[addrs.ConfigResource]()
resourceGraph.AddDependency(instCAddr.ConfigResource(), instBAddr.ConfigResource())
resourceGraph.AddDependency(instCAddr.ConfigResource(), instAAddr.ConfigResource())
deferred := NewDeferred(resourceGraph)

// Before we report anything, all three addresses should indicate that
// they don't need to have their actions deferred.
t.Run("without any deferrals yet", func(t *testing.T) {
for _, instAddr := range []addrs.AbsResourceInstance{instAAddr, instBAddr, instCAddr} {
if deferred.ShouldDeferResourceInstanceChanges(instAddr) {
t.Errorf("%s reported as needing deferred; should not be, yet", instAddr)
}
}
})

// Resource A hasn't been expanded fully, so is deferred.
deferred.ReportResourceExpansionDeferred(instAPartial, cty.DynamicVal)

t.Run("with one resource instance deferred", func(t *testing.T) {
if !deferred.ShouldDeferResourceInstanceChanges(instCAddr) {
t.Errorf("%s was not reported as needing deferred; should be deferred", instCAddr)
}
if deferred.ShouldDeferResourceInstanceChanges(instBAddr) {
t.Errorf("%s reported as needing deferred; should not be", instCAddr)
}
})
}
10 changes: 10 additions & 0 deletions internal/plans/deferring/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

// Package deferring deals with the problem of keeping track of
// "deferred actions", which means the situation where the planning of some
// objects is currently impossible due to incomplete information and so
// Terraform explicitly defers dealing with them until the next plan/apply
// round, while still allowing the operator to apply the partial plan so
// that there will be more information available next time.
package deferring
2 changes: 1 addition & 1 deletion internal/rpcapi/internal_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type Client struct {
func NewInternalClient(ctx context.Context, clientCaps *terraform1.ClientCapabilities) (*Client, error) {
fakeListener := bufconn.Listen(4 * 1024 * 1024 /* buffer size */)
srv := grpc.NewServer()
registerGRPCServices(srv)
registerGRPCServices(srv, &serviceOpts{})

go func() {
if err := srv.Serve(fakeListener); err != nil {
Expand Down
22 changes: 17 additions & 5 deletions internal/rpcapi/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,22 @@ func (p *corePlugin) GRPCClient(ctx context.Context, broker *plugin.GRPCBroker,
}

func (p *corePlugin) GRPCServer(broker *plugin.GRPCBroker, s *grpc.Server) error {
registerGRPCServices(s)
generalOpts := &serviceOpts{
experimentsAllowed: p.experimentsAllowed,
}
registerGRPCServices(s, generalOpts)
return nil
}

func registerGRPCServices(s *grpc.Server) {
func registerGRPCServices(s *grpc.Server, opts *serviceOpts) {
// We initially only register the setup server, because the registration
// of other services can vary depending on the capabilities negotiated
// during handshake.
setup := newSetupServer(serverHandshake(s))
setup := newSetupServer(serverHandshake(s, opts))
terraform1.RegisterSetupServer(s, setup)
}

func serverHandshake(s *grpc.Server) func(context.Context, *terraform1.ClientCapabilities) (*terraform1.ServerCapabilities, error) {
func serverHandshake(s *grpc.Server, opts *serviceOpts) func(context.Context, *terraform1.ClientCapabilities) (*terraform1.ServerCapabilities, error) {
dependencies := dynrpcserver.NewDependenciesStub()
terraform1.RegisterDependenciesServer(s, dependencies)
stacks := dynrpcserver.NewStacksStub()
Expand Down Expand Up @@ -71,11 +74,20 @@ func serverHandshake(s *grpc.Server) func(context.Context, *terraform1.ClientCap
// doing real work. In future the details of what we register here
// might vary based on the negotiated capabilities.
dependencies.ActivateRPCServer(newDependenciesServer(handles, services))
stacks.ActivateRPCServer(newStacksServer(handles))
stacks.ActivateRPCServer(newStacksServer(handles, opts))
packages.ActivateRPCServer(newPackagesServer(services))

// If the client requested any extra capabililties that we're going
// to honor then we should announce them in this result.
return &terraform1.ServerCapabilities{}, nil
}
}

// serviceOpts are options that could potentially apply to all of our
// individual RPC services.
//
// This could potentially be embedded inside a service-specific options
// structure, if needed.
type serviceOpts struct {
experimentsAllowed bool
}
Loading
Loading