Skip to content

Commit

Permalink
configs/configschema: Introduce the NestingGroup mode for blocks
Browse files Browse the repository at this point in the history
In study of existing providers we've found a pattern we werent previously
accounting for of using a nested block type to represent a group of
arguments that relate to a particular feature that is always enabled but
where it improves configuration readability to group all of its settings
together in a nested block.

The existing NestingSingle was not a good fit for this because it is
designed under the assumption that the presence or absence of the block
has some significance in enabling or disabling the relevant feature, and
so for these always-active cases we'd generate a misleading plan where
the settings for the feature appear totally absent, rather than showing
the default values that will be selected.

NestingGroup is, therefore, a slight variation of NestingSingle where
presence vs. absence of the block is not distinguishable (it's never null)
and instead its contents are treated as unset when the block is absent.
This then in turn causes any default values associated with the nested
arguments to be honored and displayed in the plan whenever the block is
not explicitly configured.

The current SDK cannot activate this mode, but that's okay because its
"legacy type system" opt-out flag allows it to force a block to be
processed in this way anyway. We're adding this now so that we can
introduce the feature in a future SDK without causing a breaking change
to the protocol, since the set of possible block nesting modes is not
extensible.
  • Loading branch information
apparentlymart committed Apr 10, 2019
1 parent a20084d commit 88e76fa
Show file tree
Hide file tree
Showing 25 changed files with 392 additions and 256 deletions.
7 changes: 4 additions & 3 deletions command/format/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,19 +278,20 @@ func (p *blockBodyDiffPrinter) writeNestedBlockDiffs(name string, blockS *config
// the objects within are computed.

switch blockS.Nesting {
case configschema.NestingSingle:
case configschema.NestingSingle, configschema.NestingGroup:
var action plans.Action
eqV := new.Equals(old)
switch {
case old.IsNull():
action = plans.Create
case new.IsNull():
action = plans.Delete
case !new.IsKnown() || !old.IsKnown():
case !new.IsWhollyKnown() || !old.IsWhollyKnown():
// "old" should actually always be known due to our contract
// that old values must never be unknown, but we'll allow it
// anyway to be robust.
action = plans.Update
case !(new.Equals(old).True()):
case !eqV.IsKnown() || !eqV.True():
action = plans.Update
}

Expand Down
2 changes: 1 addition & 1 deletion command/jsonconfig/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func marshalExpressions(body hcl.Body, schema *configschema.Block) expressions {
}

switch blockS.Nesting {
case configschema.NestingSingle:
case configschema.NestingSingle, configschema.NestingGroup:
ret[typeName] = marshalExpressions(block.Body, &blockS.Block)
case configschema.NestingList, configschema.NestingSet:
if _, exists := ret[typeName]; !exists {
Expand Down
2 changes: 2 additions & 0 deletions command/jsonprovider/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ func marshalBlockTypes(nestedBlock *configschema.NestedBlock) *blockType {
switch nestedBlock.Nesting {
case configschema.NestingSingle:
ret.NestingMode = "single"
case configschema.NestingGroup:
ret.NestingMode = "group"
case configschema.NestingList:
ret.NestingMode = "list"
case configschema.NestingSet:
Expand Down
2 changes: 1 addition & 1 deletion config/hcl2shim/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func ConfigValueFromHCL2Block(v cty.Value, schema *configschema.Block) map[strin

switch blockS.Nesting {

case configschema.NestingSingle:
case configschema.NestingSingle, configschema.NestingGroup:
ret[name] = ConfigValueFromHCL2Block(bv, &blockS.Block)

case configschema.NestingList, configschema.NestingSet:
Expand Down
8 changes: 6 additions & 2 deletions configs/configschema/coerce_value.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (b *Block) coerceValue(in cty.Value, path cty.Path) (cty.Value, error) {
for typeName, blockS := range b.BlockTypes {
switch blockS.Nesting {

case NestingSingle:
case NestingSingle, NestingGroup:
switch {
case ty.HasAttribute(typeName):
var err error
Expand All @@ -84,7 +84,11 @@ func (b *Block) coerceValue(in cty.Value, path cty.Path) (cty.Value, error) {
return cty.UnknownVal(b.ImpliedType()), err
}
case blockS.MinItems != 1 && blockS.MaxItems != 1:
attrs[typeName] = cty.NullVal(blockS.ImpliedType())
if blockS.Nesting == NestingGroup {
attrs[typeName] = blockS.EmptyValue()
} else {
attrs[typeName] = cty.NullVal(blockS.ImpliedType())
}
default:
// We use the word "attribute" here because we're talking about
// the cty sense of that word rather than the HCL sense.
Expand Down
10 changes: 9 additions & 1 deletion configs/configschema/decoder_spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,20 @@ func (b *Block) DecoderSpec() hcldec.Spec {
childSpec := blockS.Block.DecoderSpec()

switch blockS.Nesting {
case NestingSingle:
case NestingSingle, NestingGroup:
ret[name] = &hcldec.BlockSpec{
TypeName: name,
Nested: childSpec,
Required: blockS.MinItems == 1 && blockS.MaxItems >= 1,
}
if blockS.Nesting == NestingGroup {
ret[name] = &hcldec.DefaultSpec{
Primary: ret[name],
Default: &hcldec.LiteralSpec{
Value: blockS.EmptyValue(),
},
}
}
case NestingList:
// We prefer to use a list where possible, since it makes our
// implied type more complete, but if there are any
Expand Down
2 changes: 2 additions & 0 deletions configs/configschema/empty_value.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ func (b *NestedBlock) EmptyValue() cty.Value {
switch b.Nesting {
case NestingSingle:
return cty.NullVal(b.Block.ImpliedType())
case NestingGroup:
return b.Block.EmptyValue()
case NestingList:
if ty := b.Block.ImpliedType(); ty.HasDynamicTypes() {
return cty.EmptyTupleVal
Expand Down
19 changes: 19 additions & 0 deletions configs/configschema/empty_value_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,25 @@ func TestBlockEmptyValue(t *testing.T) {
})),
}),
},
{
&Block{
BlockTypes: map[string]*NestedBlock{
"group": {
Nesting: NestingGroup,
Block: Block{
Attributes: map[string]*Attribute{
"str": {Type: cty.String, Required: true},
},
},
},
},
},
cty.ObjectVal(map[string]cty.Value{
"group": cty.ObjectVal(map[string]cty.Value{
"str": cty.NullVal(cty.String),
}),
}),
},
{
&Block{
BlockTypes: map[string]*NestedBlock{
Expand Down
4 changes: 4 additions & 0 deletions configs/configschema/internal_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ func (b *Block) internalValidate(prefix string, err error) error {
case blockS.MinItems < 0 || blockS.MinItems > 1:
err = multierror.Append(err, fmt.Errorf("%s%s: MinItems and MaxItems must be set to either 0 or 1 in NestingSingle mode", prefix, name))
}
case NestingGroup:
if blockS.MinItems != 0 || blockS.MaxItems != 0 {
err = multierror.Append(err, fmt.Errorf("%s%s: MinItems and MaxItems cannot be used in NestingGroup mode", prefix, name))
}
case NestingList, NestingSet:
if blockS.MinItems > blockS.MaxItems && blockS.MaxItems != 0 {
err = multierror.Append(err, fmt.Errorf("%s%s: MinItems must be less than or equal to MaxItems in %s mode", prefix, name, blockS.Nesting))
Expand Down
16 changes: 14 additions & 2 deletions configs/configschema/nestingmode_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 17 additions & 0 deletions configs/configschema/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,23 @@ const (
// provided directly as an object value.
NestingSingle

// NestingGroup is similar to NestingSingle in that it calls for only a
// single instance of a given block type with no labels, but it additonally
// guarantees that its result will never be null, even if the block is
// absent, and instead the nested attributes and blocks will be treated
// as absent in that case. (Any required attributes or blocks within the
// nested block are not enforced unless the block is explicitly present
// in the configuration, so they are all effectively optional when the
// block is not present.)
//
// This is useful for the situation where a remote API has a feature that
// is always enabled but has a group of settings related to that feature
// that themselves have default values. By using NestingGroup instead of
// NestingSingle in that case, generated plans will show the block as
// present even when not present in configuration, thus allowing any
// default values within to be displayed to the user.
NestingGroup

// NestingList indicates that multiple blocks of the given type are
// permitted, with no labels, and that their corresponding objects should
// be provided in a list.
Expand Down
2 changes: 1 addition & 1 deletion configs/configschema/validate_traversal.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (b *Block) StaticValidateTraversal(traversal hcl.Traversal) tfdiags.Diagnos
}

func (b *NestedBlock) staticValidateTraversal(typeName string, traversal hcl.Traversal) tfdiags.Diagnostics {
if b.Nesting == NestingSingle {
if b.Nesting == NestingSingle || b.Nesting == NestingGroup {
// Single blocks are easy: just pass right through.
return b.Block.StaticValidateTraversal(traversal)
}
Expand Down
2 changes: 1 addition & 1 deletion configs/configupgrade/upgrade_native.go
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ func printDynamicBlockBody(buf *bytes.Buffer, iterName string, schema *configsch
case configschema.NestingMap:
printAttribute(buf, "for_each", []byte(fmt.Sprintf(`lookup(%s.value, %q, {})`, iterName, name)), nil)
printAttribute(buf, "labels", []byte(fmt.Sprintf(`[%s.key]`, name)), nil)
case configschema.NestingSingle:
case configschema.NestingSingle, configschema.NestingGroup:
printAttribute(buf, "for_each", []byte(fmt.Sprintf(`lookup(%s.value, %q, null) != null ? [%s.value.%s] : []`, iterName, name, iterName, name)), nil)
default:
printAttribute(buf, "for_each", []byte(fmt.Sprintf(`lookup(%s.value, %q, [])`, iterName, name)), nil)
Expand Down
2 changes: 1 addition & 1 deletion helper/plugin/unknown.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func SetUnknowns(val cty.Value, schema *configschema.Block) cty.Value {
// This switches on the value type here, so we can correctly switch
// between Tuples/Lists and Maps/Objects.
switch {
case blockS.Nesting == configschema.NestingSingle:
case blockS.Nesting == configschema.NestingSingle || blockS.Nesting == configschema.NestingGroup:
// NestingSingle is the only exception here, where we treat the
// block directly as an object
newVals[name] = SetUnknowns(blockVal, &blockS.Block)
Expand Down
Loading

0 comments on commit 88e76fa

Please sign in to comment.