Skip to content

Commit

Permalink
Fixed incorrect handling of missing config with optional args (#80)
Browse files Browse the repository at this point in the history
  • Loading branch information
Andrew-Morozko authored Feb 12, 2024
1 parent 45dac35 commit 55accbb
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 25 deletions.
23 changes: 4 additions & 19 deletions parser/caller.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,11 @@ func (c *Caller) callPlugin(kind, name string, config evaluation.Configuration,
return
}

// TODO: check that nil interface values are checked like this everywhere
needsConfig := !utils.IsNil(data.ConfigSpec)
hasConfig := !utils.IsNil(config)
acceptsConfig := !utils.IsNil(data.ConfigSpec)
hasConfig := config.Exists()

var configVal cty.Value
switch {
case needsConfig && hasConfig: // happy path
if acceptsConfig {
var stdDiag diagnostics.Diag
configVal, stdDiag = config.ParseConfig(data.ConfigSpec)
if !diags.Extend(stdDiag) {
Expand All @@ -105,20 +103,7 @@ func (c *Caller) callPlugin(kind, name string, config evaluation.Configuration,
}
}
}
case !needsConfig && !hasConfig:
// happy path, do nothing
case needsConfig && !hasConfig:
diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Plugin requires configuration",
Detail: fmt.Sprintf("Plugin '%s %s' has no default configuration and "+
"no configuration was provided at the plugin invocation. "+
"Provide an inline config block or a config attribute",
kind, name),
Subject: invocation.MissingItemRange().Ptr(),
Context: invocation.Range().Ptr(),
})
case !needsConfig && hasConfig:
} else if hasConfig {
diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagWarning,
Summary: "Plugin doesn't support configuration",
Expand Down
8 changes: 8 additions & 0 deletions parser/definedBlocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,14 @@ func (db *DefinedBlocks) GetConfig(expr hcl.Expression) (cfg *definitions.Config
return
}

func (db *DefinedBlocks) DefaultConfigFor(plugin *definitions.Plugin) (config *definitions.Config) {
return db.Config[definitions.Key{
PluginKind: plugin.Kind(),
PluginName: plugin.Name(),
BlockName: "",
}]
}

func (db *DefinedBlocks) Merge(other *DefinedBlocks) (diags diagnostics.Diag) {
for k, v := range other.Config {
diags.Append(AddIfMissing(db.Config, k, v))
Expand Down
5 changes: 5 additions & 0 deletions parser/definitions/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ type Config struct {
value cty.Value
}

// Exists implements evaluation.Configuration.
func (c *Config) Exists() bool {
return c != nil
}

var _ evaluation.Configuration = (*Config)(nil)

// ParseConfig implements Configuration.
Expand Down
49 changes: 49 additions & 0 deletions parser/definitions/configEmpty.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package definitions

import (
"fmt"

"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hcldec"
"github.com/hashicorp/hcl/v2/hclsyntax"
"github.com/zclconf/go-cty/cty"

"github.com/blackstork-io/fabric/parser/evaluation"
"github.com/blackstork-io/fabric/pkg/diagnostics"
)

// Empty config, storing the range of the original block
type ConfigEmpty struct {
MissingItemRange hcl.Range
}

// Exists implements evaluation.Configuration.
func (c *ConfigEmpty) Exists() bool {
return false
}

// ParseConfig implements Configuration.
func (c *ConfigEmpty) ParseConfig(spec hcldec.Spec) (val cty.Value, diags diagnostics.Diag) {
emptyBody := &hclsyntax.Body{
SrcRange: c.MissingItemRange,
EndRange: hcl.Range{
Filename: c.MissingItemRange.Filename,
Start: c.MissingItemRange.End,
End: c.MissingItemRange.End,
},
}

var diag hcl.Diagnostics
val, diag = hcldec.Decode(emptyBody, spec, nil)
for _, d := range diag {
d.Summary = fmt.Sprintf("Missing required configuration: %s", d.Summary)
}
return val, diagnostics.Diag(diag)
}

// Range implements Configuration.
func (c *ConfigEmpty) Range() hcl.Range {
return c.MissingItemRange
}

var _ evaluation.Configuration = (*ConfigEmpty)(nil)
5 changes: 5 additions & 0 deletions parser/definitions/configPtr.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ type ConfigPtr struct {
Ptr *hcl.Attribute
}

// Exists implements evaluation.Configuration.
func (c *ConfigPtr) Exists() bool {
return c != nil
}

// ParseConfig implements Configuration.
func (c *ConfigPtr) ParseConfig(spec hcldec.Spec) (val cty.Value, diags diagnostics.Diag) {
return c.Cfg.ParseConfig(spec)
Expand Down
1 change: 1 addition & 0 deletions parser/evaluation/evaluation.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
type Configuration interface {
ParseConfig(spec hcldec.Spec) (cty.Value, diagnostics.Diag)
Range() hcl.Range
Exists() bool
}

// To act as a plugin invocation (body of the plugin call block)
Expand Down
12 changes: 6 additions & 6 deletions parser/parsePluginBlock.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,13 @@ func (db *DefinedBlocks) parsePluginConfig(plugin *definitions.Plugin, configAtt
}
} else if plugin.IsRef() {
config = refBaseConfig
} else {
} else if defaultCfg := db.DefaultConfigFor(plugin); defaultCfg != nil {
// Apply default configs to non-refs only
config = db.Config[definitions.Key{
PluginKind: plugin.Kind(),
PluginName: plugin.Name(),
BlockName: "",
}]
config = defaultCfg
} else {
config = &definitions.ConfigEmpty{
MissingItemRange: plugin.Block.Body.MissingItemRange(),
}
}
return
}
Expand Down

0 comments on commit 55accbb

Please sign in to comment.