From 98aebab54061705cb9745ca237270c8ce93f3403 Mon Sep 17 00:00:00 2001 From: Dmitry Moskowski Date: Sun, 14 Aug 2022 16:12:39 +0000 Subject: [PATCH] fixing bastions & settings merge semantics --- provider/provider.go | 62 +++++++++++++++-------------------- provider/schema_test.go | 71 ++++++++++++++++++++++++++++++----------- provider/ssh.go | 16 ++++++++-- test/main.tf | 18 ++++------- 4 files changed, 98 insertions(+), 69 deletions(-) diff --git a/provider/provider.go b/provider/provider.go index 3588b6e..654ad10 100644 --- a/provider/provider.go +++ b/provider/provider.go @@ -127,6 +127,9 @@ func (p *Provider) resolveSettings(r schemaResource, path ...string) interface{} for _, key := range path { value = fold(key, value) + if value == nil { + return nil + } } // @@ -148,21 +151,11 @@ func (p *Provider) merge(resources ...map[string]interface{}) map[string]interfa // - what fields have default values // - what field have been provided by user // But this garbage SDK provides no way to know about this, - // making default values are indistinguishable from legitimate values - // providen by the user. - // For example, this is why when you override some fields of bastion on - // per-instance basis you would get some fields nulled. for _, resource := range resources { for key, value := range resource { rvalue := reflect.ValueOf(value) switch rvalue.Kind() { - case - reflect.String, - reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, - reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, - reflect.Float32, reflect.Float64: - r[key] = value case reflect.Map: current, ok := r[key] if ok { @@ -186,14 +179,12 @@ func (p *Provider) merge(resources ...map[string]interface{}) map[string]interfa func (p *Provider) settings(resource ResourceBox, path ...string) map[string]interface{} { providerLevel := map[string]interface{}{} settings, _ := p.resolveSettings(p, path...).([]interface{}) - if len(settings) == 0 { - return providerLevel + if len(settings) != 0 { + providerLevel = settings[0].(map[string]interface{}) } - providerLevel = settings[0].(map[string]interface{}) if resource == nil { return providerLevel } - resourceLevelSettings, _ := p.resolveSettings(resource, path...).([]interface{}) if len(resourceLevelSettings) > 0 { resourceLevel := resourceLevelSettings[0].(map[string]interface{}) @@ -310,35 +301,32 @@ func (p *Provider) NewSsh(resource ResourceBox) *Ssh { bastionSettings = p.SshBastionSettings(resource) ) - if len(bastionSettings) > 0 { - bastionHost, _ := bastionSettings[KeySshHost].(string) - if bastionHost != "" { - bastionConfigMap := p.SshConfigMap(bastionSettings) - if bastionConfigMap.Len() > 0 { - bastionConfigOption := SshOptionConfigMap(bastionConfigMap.Pairs()) - bastion := NewSsh( - bastionConfigOption, - SshOptionNonInteractive(), - SshOptionIORedirection("%h", "%p"), - SshOptionHost(bastionHost), - ) - command, arguments, _ := bastion.Command() - configMap.Set( - SshConfigKeyProxyCommand, - strings.Join(append([]string{command}, arguments...), " "), - ) - options = append( - options, - bastionConfigOption, - ) - } + bastionHost, _ := bastionSettings[KeySshHost].(string) + if bastionHost != "" { + bastionConfigMap := p.SshConfigMap(bastionSettings) + if bastionConfigMap.Len() > 0 { + // NOTE: base ssh configuration (ssh {}) extended with bastion ssh configuration (ssh { bastion {} }) + extendedBastionConfigMap := configMap.Copy() + extendedBastionConfigMap.Extend(bastionConfigMap) + + bastion := NewSsh( + SshOptionConfigMap(extendedBastionConfigMap), + SshOptionNonInteractive(), + SshOptionIORedirection("%h", "%p"), + SshOptionHost(bastionHost), + ) + command, arguments, _ := bastion.Command() + configMap.Set( + SshConfigKeyProxyCommand, + strings.Join(append([]string{command}, arguments...), " "), + ) } } if configMap.Len() > 0 { options = append( options, - SshOptionConfigMap(configMap.Pairs()), + SshOptionConfigMap(configMap), ) } diff --git a/provider/schema_test.go b/provider/schema_test.go index 5403edc..f94a507 100644 --- a/provider/schema_test.go +++ b/provider/schema_test.go @@ -16,7 +16,7 @@ var providerFactories = map[string]func() (*schema.Provider, error){ }, } -const nixosMinimalConfig = ` +const nixosConfig1 = ` provider "nixos" { nix { activation_action = "" # skip activation because we are running in docker @@ -32,12 +32,38 @@ provider "nixos" { } } -resource "nixos_instance" "test" { +resource "nixos_instance" "test1" { address = ["127.0.0.1", "::1"] configuration = "../test/test.nix" } ` -const nixosSampleConfig = ` +const nixosConfig2 = ` +provider "nixos" { + nix { + activation_action = "" # skip activation because we are running in docker + } +} + +resource "nixos_instance" "test2" { + address = ["127.0.0.1", "::1"] + configuration = "../test/test.nix" + + ssh { + port = 2222 + config = { + userKnownHostsFile = "/dev/null" + strictHostKeyChecking = "no" + pubKeyAuthentication = "no" + passwordAuthentication = "yes" + } + bastion { + host = "127.0.0.1" + port = 2222 + } + } +} +` +const nixosConfig3 = ` provider "nixos" { retry = 0 nix { @@ -45,7 +71,7 @@ provider "nixos" { activation_action = "" # skip activation because we are running in docker } ssh { - port = 666 + port = 2222 config = { userKnownHostsFile = "/dev/null" strictHostKeyChecking = "no" @@ -76,12 +102,9 @@ provider "nixos" { } } -resource "nixos_instance" "test" { +resource "nixos_instance" "test3" { address = ["127.0.0.1", "::1"] configuration = "../test/test.nix" - ssh { - port = 2222 - } } ` @@ -113,21 +136,33 @@ func TestResourceNixosInstance(t *testing.T) { ProviderFactories: providerFactories, Steps: []resource.TestStep{ { - Config: nixosMinimalConfig, + Config: nixosConfig1, + Check: resource.ComposeTestCheckFunc( + CheckEqual(t, "nixos_instance.test1", "address.0", "127.0.0.1"), + CheckEqual(t, "nixos_instance.test1", "address.1", "::1"), + CheckEqual(t, "nixos_instance.test1", "address.2", ""), + CheckEqual(t, "nixos_instance.test1", "configuration", "../test/test.nix"), + ), + }, + { + Config: nixosConfig2, Check: resource.ComposeTestCheckFunc( - CheckEqual(t, "nixos_instance.test", "address.0", "127.0.0.1"), - CheckEqual(t, "nixos_instance.test", "address.1", "::1"), - CheckEqual(t, "nixos_instance.test", "address.2", ""), - CheckEqual(t, "nixos_instance.test", "configuration", "../test/test.nix"), + CheckEqual(t, "nixos_instance.test2", "address.0", "127.0.0.1"), + CheckEqual(t, "nixos_instance.test2", "address.1", "::1"), + CheckEqual(t, "nixos_instance.test2", "address.2", ""), + CheckEqual(t, "nixos_instance.test2", "configuration", "../test/test.nix"), + CheckEqual(t, "nixos_instance.test2", "ssh.0.port", "2222"), + CheckEqual(t, "nixos_instance.test2", "ssh.0.bastion.0.host", "127.0.0.1"), + CheckEqual(t, "nixos_instance.test2", "ssh.0.bastion.0.port", "2222"), ), }, { - Config: nixosSampleConfig, + Config: nixosConfig3, Check: resource.ComposeTestCheckFunc( - CheckEqual(t, "nixos_instance.test", "address.0", "127.0.0.1"), - CheckEqual(t, "nixos_instance.test", "address.1", "::1"), - CheckEqual(t, "nixos_instance.test", "address.2", ""), - CheckEqual(t, "nixos_instance.test", "configuration", "../test/test.nix"), + CheckEqual(t, "nixos_instance.test3", "address.0", "127.0.0.1"), + CheckEqual(t, "nixos_instance.test3", "address.1", "::1"), + CheckEqual(t, "nixos_instance.test3", "address.2", ""), + CheckEqual(t, "nixos_instance.test3", "configuration", "../test/test.nix"), ), }, }, diff --git a/provider/ssh.go b/provider/ssh.go index 39cb1f0..d095c8d 100644 --- a/provider/ssh.go +++ b/provider/ssh.go @@ -44,6 +44,18 @@ func (m *SshConfigMap) Get(key string) (string, bool) { return m.store[i].Value, true } +func (m *SshConfigMap) Extend(em *SshConfigMap) { + for _, pair := range em.Pairs() { + m.Set(pair.Key, pair.Value) + } +} + +func (m *SshConfigMap) Copy() *SshConfigMap { + nm := NewSshConfigMap() + nm.Extend(m) + return nm +} + func (m *SshConfigMap) Len() int { return len(m.store) } @@ -90,13 +102,13 @@ func SshOptionConfigFile(fd File) SshOption { return SshOptionConfig(fd.Name()) } -func SshOptionConfigMap(ps SshConfigPairs) SshOption { +func SshOptionConfigMap(m *SshConfigMap) SshOption { return func(s *Ssh) { fd, err := CreateTemp("ssh_config.*") if err != nil { panic(err) } - _, err = fd.Write([]byte(SshSerializeConfig(ps))) + _, err = fd.Write([]byte(SshSerializeConfig(m.Pairs()))) if err != nil { panic(err) } diff --git a/test/main.tf b/test/main.tf index b4ea32d..54b4461 100644 --- a/test/main.tf +++ b/test/main.tf @@ -22,12 +22,6 @@ provider "nixos" { bastion { host = "127.0.0.1" port = 777 - config = { - userKnownHostsFile = "/dev/null" - strictHostKeyChecking = "no" - pubKeyAuthentication = "no" - passwordAuthentication = "yes" - } } } } @@ -41,15 +35,15 @@ resource "nixos_instance" "test" { } ssh { port = 2222 + config = { + userKnownHostsFile = "/dev/null" + strictHostKeyChecking = "no" + pubKeyAuthentication = "no" + passwordAuthentication = "yes" + } bastion { host = "127.0.0.1" port = 2222 - config = { - userKnownHostsFile = "/dev/null" - strictHostKeyChecking = "no" - pubKeyAuthentication = "no" - passwordAuthentication = "yes" - } } }