Skip to content

Commit 36125be

Browse files
tobiotobio
committed
PR feedback
1 parent e6aecc0 commit 36125be

File tree

6 files changed

+784
-34
lines changed

6 files changed

+784
-34
lines changed

‎internal/clients/config/elasticsearch.go

+13-14
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010

1111
"github.com./elastic/go-elasticsearch/v7"
1212
fwdiags "github.com./hashicorp/terraform-plugin-framework/diag"
13-
"github.com./hashicorp/terraform-plugin-sdk/v2/diag"
1413
sdkdiags "github.com./hashicorp/terraform-plugin-sdk/v2/diag"
1514
"github.com./hashicorp/terraform-plugin-sdk/v2/helper/logging"
1615
"github.com./hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -24,7 +23,7 @@ func newElasticsearchConfigFromSDK(d *schema.ResourceData, base baseConfig, key
2423
return nil, nil
2524
}
2625

27-
var diags diag.Diagnostics
26+
var diags sdkdiags.Diagnostics
2827
config := base.toElasticsearchConfig()
2928

3029
// if defined, then we only have a single entry
@@ -47,8 +46,8 @@ func newElasticsearchConfigFromSDK(d *schema.ResourceData, base baseConfig, key
4746
if caFile, ok := esConfig["ca_file"]; ok && caFile.(string) != "" {
4847
caCert, err := os.ReadFile(caFile.(string))
4948
if err != nil {
50-
diags = append(diags, diag.Diagnostic{
51-
Severity: diag.Error,
49+
diags = append(diags, sdkdiags.Diagnostic{
50+
Severity: sdkdiags.Error,
5251
Summary: "Unable to read CA File",
5352
Detail: err.Error(),
5453
})
@@ -64,8 +63,8 @@ func newElasticsearchConfigFromSDK(d *schema.ResourceData, base baseConfig, key
6463
if keyFile, ok := esConfig["key_file"]; ok && keyFile.(string) != "" {
6564
cert, err := tls.LoadX509KeyPair(certFile.(string), keyFile.(string))
6665
if err != nil {
67-
diags = append(diags, diag.Diagnostic{
68-
Severity: diag.Error,
66+
diags = append(diags, sdkdiags.Diagnostic{
67+
Severity: sdkdiags.Error,
6968
Summary: "Unable to read certificate or key file",
7069
Detail: err.Error(),
7170
})
@@ -74,8 +73,8 @@ func newElasticsearchConfigFromSDK(d *schema.ResourceData, base baseConfig, key
7473
tlsClientConfig := config.ensureTLSClientConfig()
7574
tlsClientConfig.Certificates = []tls.Certificate{cert}
7675
} else {
77-
diags = append(diags, diag.Diagnostic{
78-
Severity: diag.Error,
76+
diags = append(diags, sdkdiags.Diagnostic{
77+
Severity: sdkdiags.Error,
7978
Summary: "Unable to read key file",
8079
Detail: "Path to key file has not been configured or is empty",
8180
})
@@ -86,8 +85,8 @@ func newElasticsearchConfigFromSDK(d *schema.ResourceData, base baseConfig, key
8685
if keyData, ok := esConfig["key_data"]; ok && keyData.(string) != "" {
8786
cert, err := tls.X509KeyPair([]byte(certData.(string)), []byte(keyData.(string)))
8887
if err != nil {
89-
diags = append(diags, diag.Diagnostic{
90-
Severity: diag.Error,
88+
diags = append(diags, sdkdiags.Diagnostic{
89+
Severity: sdkdiags.Error,
9190
Summary: "Unable to parse certificate or key",
9291
Detail: err.Error(),
9392
})
@@ -96,8 +95,8 @@ func newElasticsearchConfigFromSDK(d *schema.ResourceData, base baseConfig, key
9695
tlsClientConfig := config.ensureTLSClientConfig()
9796
tlsClientConfig.Certificates = []tls.Certificate{cert}
9897
} else {
99-
diags = append(diags, diag.Diagnostic{
100-
Severity: diag.Error,
98+
diags = append(diags, sdkdiags.Diagnostic{
99+
Severity: sdkdiags.Error,
101100
Summary: "Unable to parse key",
102101
Detail: "Key data has not been configured or is empty",
103102
})
@@ -208,9 +207,9 @@ func (c elasticsearchConfig) withEnvironmentOverrides() elasticsearchConfig {
208207
}
209208

210209
if insecure, ok := os.LookupEnv("ELASTICSEARCH_INSECURE"); ok {
211-
if insecureValue, _ := strconv.ParseBool(insecure); insecureValue {
210+
if insecureValue, err := strconv.ParseBool(insecure); err != nil {
212211
tlsClientConfig := c.ensureTLSClientConfig()
213-
tlsClientConfig.InsecureSkipVerify = true
212+
tlsClientConfig.InsecureSkipVerify = insecureValue
214213
}
215214
}
216215

‎internal/clients/config/elasticsearch_test.go

+229
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
package config
2+
3+
import (
4+
"context"
5+
"os"
6+
"testing"
7+
8+
"github.com./hashicorp/terraform-plugin-framework/attr"
9+
fwdiags "github.com./hashicorp/terraform-plugin-framework/diag"
10+
"github.com./hashicorp/terraform-plugin-framework/types/basetypes"
11+
sdkdiags "github.com./hashicorp/terraform-plugin-sdk/v2/diag"
12+
"github.com./hashicorp/terraform-plugin-sdk/v2/helper/schema"
13+
14+
providerSchema "github.com./elastic/terraform-provider-elasticstack/internal/schema"
15+
"github.com./elastic/terraform-provider-elasticstack/internal/utils"
16+
"github.com./stretchr/testify/require"
17+
)
18+
19+
func Test_newElasticsearchConfigFromSDK(t *testing.T) {
20+
type args struct {
21+
resourceData map[string]interface{}
22+
base baseConfig
23+
env map[string]string
24+
expectedESConfig *elasticsearchConfig
25+
expectedDiags sdkdiags.Diagnostics
26+
}
27+
tests := []struct {
28+
name string
29+
args func(string) args
30+
}{
31+
{
32+
name: "should return nil if no config is specified",
33+
args: func(key string) args {
34+
return args{}
35+
},
36+
},
37+
{
38+
name: "should use the options set in config",
39+
args: func(key string) args {
40+
base := baseConfig{
41+
Username: "elastic",
42+
Password: "changeme",
43+
}
44+
45+
config := base.toElasticsearchConfig()
46+
config.Addresses = []string{"localhost", "example.com"}
47+
tlsConfig := config.ensureTLSClientConfig()
48+
tlsConfig.InsecureSkipVerify = true
49+
50+
return args{
51+
resourceData: map[string]interface{}{
52+
key: []interface{}{
53+
map[string]interface{}{
54+
"endpoints": []interface{}{"localhost", "example.com"},
55+
"insecure": true,
56+
},
57+
},
58+
},
59+
base: base,
60+
expectedESConfig: &config,
61+
}
62+
},
63+
},
64+
{
65+
name: "should prefer config defined in environment variables",
66+
args: func(key string) args {
67+
base := baseConfig{
68+
Username: "elastic",
69+
Password: "changeme",
70+
}
71+
72+
config := base.toElasticsearchConfig()
73+
config.Addresses = []string{"127.0.0.1", "example.com/elastic"}
74+
tlsConfig := config.ensureTLSClientConfig()
75+
tlsConfig.InsecureSkipVerify = false
76+
77+
return args{
78+
resourceData: map[string]interface{}{
79+
key: []interface{}{
80+
map[string]interface{}{
81+
"endpoints": []interface{}{"localhost", "example.com"},
82+
"insecure": true,
83+
},
84+
},
85+
},
86+
env: map[string]string{
87+
"ELASTICSEARCH_ENDPOINTS": "127.0.0.1,example.com/elastic",
88+
"ELASTICSEARCH_INSECURE": "false",
89+
},
90+
base: base,
91+
expectedESConfig: &config,
92+
}
93+
},
94+
},
95+
}
96+
97+
for _, tt := range tests {
98+
t.Run(tt.name, func(t *testing.T) {
99+
os.Unsetenv("ELASTICSEARCH_ENDPOINTS")
100+
os.Unsetenv("ELASTICSEARCH_INSECURE")
101+
102+
key := "elasticsearch"
103+
args := tt.args(key)
104+
rd := schema.TestResourceDataRaw(t, map[string]*schema.Schema{
105+
key: providerSchema.GetEsConnectionSchema(key, true),
106+
}, args.resourceData)
107+
108+
for key, val := range args.env {
109+
os.Setenv(key, val)
110+
}
111+
112+
esConfig, diags := newElasticsearchConfigFromSDK(rd, args.base, key, false)
113+
114+
require.Equal(t, args.expectedESConfig, esConfig)
115+
require.Equal(t, args.expectedDiags, diags)
116+
})
117+
}
118+
}
119+
120+
func Test_newElasticsearchConfigFromFramework(t *testing.T) {
121+
type args struct {
122+
providerConfig ProviderConfiguration
123+
base baseConfig
124+
env map[string]string
125+
expectedESConfig *elasticsearchConfig
126+
expectedDiags fwdiags.Diagnostics
127+
}
128+
tests := []struct {
129+
name string
130+
args func() args
131+
}{
132+
{
133+
name: "should return nil if no config is specified",
134+
args: func() args {
135+
return args{
136+
providerConfig: ProviderConfiguration{},
137+
}
138+
},
139+
},
140+
{
141+
name: "should use the options set in config",
142+
args: func() args {
143+
base := baseConfig{
144+
Username: "elastic",
145+
Password: "changeme",
146+
}
147+
148+
config := base.toElasticsearchConfig()
149+
config.Addresses = []string{"localhost", "example.com"}
150+
tlsConfig := config.ensureTLSClientConfig()
151+
tlsConfig.InsecureSkipVerify = true
152+
153+
return args{
154+
providerConfig: ProviderConfiguration{
155+
Elasticsearch: []ElasticsearchConnection{
156+
{
157+
Endpoints: basetypes.NewListValueMust(
158+
basetypes.StringType{},
159+
[]attr.Value{
160+
basetypes.NewStringValue("localhost"),
161+
basetypes.NewStringValue("example.com"),
162+
},
163+
),
164+
Insecure: basetypes.NewBoolPointerValue(utils.Pointer(true)),
165+
},
166+
},
167+
},
168+
base: base,
169+
expectedESConfig: &config,
170+
}
171+
},
172+
},
173+
{
174+
name: "should prefer config defined in environment variables",
175+
args: func() args {
176+
base := baseConfig{
177+
Username: "elastic",
178+
Password: "changeme",
179+
}
180+
181+
config := base.toElasticsearchConfig()
182+
config.Addresses = []string{"127.0.0.1", "example.com/elastic"}
183+
tlsConfig := config.ensureTLSClientConfig()
184+
tlsConfig.InsecureSkipVerify = false
185+
186+
return args{
187+
providerConfig: ProviderConfiguration{
188+
Elasticsearch: []ElasticsearchConnection{
189+
{
190+
Endpoints: basetypes.NewListValueMust(
191+
basetypes.StringType{},
192+
[]attr.Value{
193+
basetypes.NewStringValue("localhost"),
194+
basetypes.NewStringValue("example.com"),
195+
},
196+
),
197+
Insecure: basetypes.NewBoolPointerValue(utils.Pointer(true)),
198+
},
199+
},
200+
},
201+
env: map[string]string{
202+
"ELASTICSEARCH_ENDPOINTS": "127.0.0.1,example.com/elastic",
203+
"ELASTICSEARCH_INSECURE": "false",
204+
},
205+
base: base,
206+
expectedESConfig: &config,
207+
}
208+
},
209+
},
210+
}
211+
212+
for _, tt := range tests {
213+
t.Run(tt.name, func(t *testing.T) {
214+
os.Unsetenv("ELASTICSEARCH_ENDPOINTS")
215+
os.Unsetenv("ELASTICSEARCH_INSECURE")
216+
217+
args := tt.args()
218+
219+
for key, val := range args.env {
220+
os.Setenv(key, val)
221+
}
222+
223+
esConfig, diags := newElasticsearchConfigFromFramework(context.Background(), args.providerConfig, args.base)
224+
225+
require.Equal(t, args.expectedESConfig, esConfig)
226+
require.Equal(t, args.expectedDiags, diags)
227+
})
228+
}
229+
}

0 commit comments

Comments
 (0)