Skip to content

Commit d6ddbe9

Browse files
kangastakangasta
authored
refactor: use credentials module to parse credentials (#101)
* refactor: use `credentials` module to parse credentials * refactor: use `driver.CredentialsFromEnv` in acc tests
1 parent e63faae commit d6ddbe9

File tree

10 files changed

+124
-159
lines changed

10 files changed

+124
-159
lines changed

‎CHANGELOG.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,14 @@ See updating [Changelog example here](https://keepachangelog.com/en/1.0.0/)
55

66
## [Unreleased]
77

8+
### Added
9+
10+
- Support for using API token from system keyring.
11+
12+
### Changed
13+
14+
- If both basic authentication and API token are provided, the API token will be used instead of raising an error.
15+
816
## [1.8.1] - 2025-08-18
917

1018
### Fixed

‎builder/upcloud/builder_acc_test.go‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -177,11 +177,10 @@ func TestBuilderAcc_network_interfaces(t *testing.T) {
177177

178178
func testAccPreCheck(t *testing.T) {
179179
t.Helper()
180-
if v := driver.UsernameFromEnv(); v == "" {
181-
t.Skipf("%s or %s must be set for acceptance tests", driver.EnvConfigUsernameLegacy, driver.EnvConfigUsername)
182-
}
183-
if v := driver.PasswordFromEnv(); v == "" {
184-
t.Skipf("%s or %s must be set for acceptance tests", driver.EnvConfigPasswordLegacy, driver.EnvConfigPassword)
180+
181+
_, err := driver.CredentialsFromEnv("", "", "")
182+
if err != nil {
183+
t.Skip(err.Error())
185184
}
186185
}
187186

@@ -261,9 +260,11 @@ func teardown(t *testing.T, testName string) func() error {
261260
return err
262261
}
263262

263+
creds, _ := driver.CredentialsFromEnv("", "", "")
264264
drv := driver.NewDriver(&driver.DriverConfig{
265-
Username: driver.UsernameFromEnv(),
266-
Password: driver.PasswordFromEnv(),
265+
Username: creds.Username,
266+
Password: creds.Password,
267+
Token: creds.Token,
267268
Timeout: defaultTestTimeout,
268269
})
269270

‎builder/upcloud/config.go‎

Lines changed: 13 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,11 @@ func (c *Config) Prepare(raws ...interface{}) ([]string, error) {
144144
return nil, fmt.Errorf("failed to decode configuration: %w", err)
145145
}
146146

147-
c.SetEnv()
147+
err = c.SetEnv()
148+
if err != nil {
149+
return nil, err
150+
}
151+
148152
c.SetDefaults()
149153

150154
if errs := c.validate(); errs != nil && len(errs.Errors) > 0 {
@@ -188,11 +192,6 @@ func (c *Config) validate() *packer.MultiError {
188192
errs = packer.MultiErrorAppend(errs, es...)
189193
}
190194

191-
// Validate authentication
192-
if authErrs := c.validateAuthentication(); authErrs != nil {
193-
errs = packer.MultiErrorAppend(errs, authErrs.Errors...)
194-
}
195-
196195
// Validate required fields
197196
if c.Zone == "" {
198197
errs = packer.MultiErrorAppend(
@@ -214,43 +213,6 @@ func (c *Config) validate() *packer.MultiError {
214213
return errs
215214
}
216215

217-
// validateAuthentication checks authentication configuration.
218-
func (c *Config) validateAuthentication() *packer.MultiError {
219-
var errs *packer.MultiError
220-
221-
// Check authentication: either username/password OR token, but not both
222-
hasUsernamePassword := c.Username != "" || c.Password != ""
223-
hasAPIToken := c.Token != ""
224-
225-
if hasUsernamePassword && hasAPIToken {
226-
errs = packer.MultiErrorAppend(
227-
errs, errors.New("you cannot specify both username/password and token. Use either username/password or token for authentication"),
228-
)
229-
}
230-
231-
if !hasUsernamePassword && !hasAPIToken {
232-
errs = packer.MultiErrorAppend(
233-
errs, errors.New("authentication required: specify either username and password, or token"),
234-
)
235-
}
236-
237-
// If using username/password, both must be provided
238-
if hasUsernamePassword && (c.Username == "" || c.Password == "") {
239-
if c.Username == "" {
240-
errs = packer.MultiErrorAppend(
241-
errs, errors.New("'username' must be specified when using username/password authentication"),
242-
)
243-
}
244-
if c.Password == "" {
245-
errs = packer.MultiErrorAppend(
246-
errs, errors.New("'password' must be specified when using username/password authentication"),
247-
)
248-
}
249-
}
250-
251-
return errs
252-
}
253-
254216
// validateTemplate checks template configuration.
255217
func (c *Config) validateTemplate() *packer.MultiError {
256218
var errs *packer.MultiError
@@ -277,16 +239,14 @@ func (c *Config) validateTemplate() *packer.MultiError {
277239
}
278240

279241
// get params from environment.
280-
func (c *Config) SetEnv() {
281-
if c.Username == "" {
282-
c.Username = driver.UsernameFromEnv()
283-
}
284-
285-
if c.Password == "" {
286-
c.Password = driver.PasswordFromEnv()
242+
func (c *Config) SetEnv() error {
243+
creds, err := driver.CredentialsFromEnv(c.Username, c.Password, c.Token)
244+
if err != nil {
245+
return err //nolint:wrapcheck // Use the original error from the shared credentials package
287246
}
288247

289-
if c.Token == "" {
290-
c.Token = driver.TokenFromEnv()
291-
}
248+
c.Username = creds.Username
249+
c.Password = creds.Password
250+
c.Token = creds.Token
251+
return nil
292252
}

‎builder/upcloud/config_test.go‎

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"time"
88

99
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
1011

1112
"github.com/UpCloudLtd/packer-plugin-upcloud/builder/upcloud"
1213
"github.com/UpCloudLtd/packer-plugin-upcloud/internal/driver"
@@ -65,8 +66,10 @@ func TestConfig_Prepare_BothAuthMethods(t *testing.T) {
6566
}
6667

6768
warns, err := c.Prepare(raws...)
68-
assert.Error(t, err)
69-
assert.Contains(t, err.Error(), "you cannot specify both username/password and token")
69+
assert.NoError(t, err)
70+
assert.Equal(t, "test-api-token", c.Token)
71+
assert.Equal(t, "", c.Username)
72+
assert.Equal(t, "", c.Password)
7073
assert.Empty(t, warns)
7174
}
7275

@@ -81,8 +84,8 @@ func TestConfig_Prepare_NoAuthMethods(t *testing.T) {
8184
}
8285

8386
warns, err := c.Prepare(raws...)
84-
assert.Error(t, err)
85-
assert.Contains(t, err.Error(), "authentication required: specify either username and password, or token")
87+
require.Error(t, err)
88+
assert.Contains(t, err.Error(), "credentials not found")
8689
assert.Empty(t, warns)
8790
}
8891

@@ -98,8 +101,8 @@ func TestConfig_Prepare_OnlyUsername(t *testing.T) {
98101
}
99102

100103
warns, err := c.Prepare(raws...)
101-
assert.Error(t, err)
102-
assert.Contains(t, err.Error(), "'password' must be specified when using username/password authentication")
104+
require.Error(t, err)
105+
assert.Contains(t, err.Error(), "credentials not found")
103106
assert.Empty(t, warns)
104107
}
105108

@@ -115,8 +118,8 @@ func TestConfig_Prepare_OnlyPassword(t *testing.T) {
115118
}
116119

117120
warns, err := c.Prepare(raws...)
118-
assert.Error(t, err)
119-
assert.Contains(t, err.Error(), "'username' must be specified when using username/password authentication")
121+
require.Error(t, err)
122+
assert.Contains(t, err.Error(), "credentials not found")
120123
assert.Empty(t, warns)
121124
}
122125

@@ -283,25 +286,45 @@ func TestConfig_setEnv_APIToken(t *testing.T) {
283286
t.Setenv(driver.EnvConfigAPIToken, "test-token")
284287

285288
c := &upcloud.Config{}
286-
c.SetEnv()
289+
err := c.SetEnv()
290+
assert.NoError(t, err)
287291
assert.Equal(t, "test-token", c.Token)
288292
}
289293

290-
func TestConfig_setEnv_DoesNotOverrideExisting(t *testing.T) {
294+
func TestConfig_setEnv_DoesNotOverrideExisting_basic(t *testing.T) {
291295
t.Setenv(driver.EnvConfigUsername, "env-user")
292296
t.Setenv(driver.EnvConfigPassword, "env-pass")
293297
t.Setenv(driver.EnvConfigAPIToken, "env-token")
294298

295299
c := &upcloud.Config{
296300
Username: "existing-user",
297301
Password: "existing-pass",
298-
Token: "existing-token",
299302
}
300-
c.SetEnv()
303+
err := c.SetEnv()
304+
assert.NoError(t, err)
301305

302306
// Should not override existing values
303307
assert.Equal(t, "existing-user", c.Username)
304308
assert.Equal(t, "existing-pass", c.Password)
309+
assert.Equal(t, "", c.Token)
310+
}
311+
312+
func TestConfig_setEnv_DoesNotOverrideExisting_token(t *testing.T) {
313+
t.Setenv(driver.EnvConfigUsername, "env-user")
314+
t.Setenv(driver.EnvConfigPassword, "env-pass")
315+
t.Setenv(driver.EnvConfigAPIToken, "env-token")
316+
317+
c := &upcloud.Config{
318+
Username: "existing-user",
319+
Password: "existing-pass",
320+
Token: "existing-token",
321+
}
322+
err := c.SetEnv()
323+
assert.NoError(t, err)
324+
325+
// Should not override existing values
326+
assert.Equal(t, "", c.Username)
327+
assert.Equal(t, "", c.Password)
305328
assert.Equal(t, "existing-token", c.Token)
306329
}
307330

‎go.mod‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ module github.com/UpCloudLtd/packer-plugin-upcloud
33
go 1.25.0
44

55
require (
6+
github.com/UpCloudLtd/upcloud-go-api/credentials v0.1.1
67
github.com/UpCloudLtd/upcloud-go-api/v8 v8.25.0
78
github.com/hashicorp/hcl/v2 v2.24.0
89
github.com/hashicorp/packer-plugin-sdk v0.6.2
@@ -12,6 +13,7 @@ require (
1213
)
1314

1415
require (
16+
al.essio.dev/pkg/shellescape v1.5.1 // indirect
1517
cloud.google.com/go v0.110.8 // indirect
1618
cloud.google.com/go/compute/metadata v0.3.0 // indirect
1719
cloud.google.com/go/iam v1.1.3 // indirect
@@ -26,10 +28,12 @@ require (
2628
github.com/aws/aws-sdk-go v1.44.114 // indirect
2729
github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect
2830
github.com/cenkalti/backoff/v3 v3.2.2 // indirect
31+
github.com/danieljoos/wincred v1.2.2 // indirect
2932
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
3033
github.com/dylanmei/iso8601 v0.1.0 // indirect
3134
github.com/fatih/color v1.16.0 // indirect
3235
github.com/go-jose/go-jose/v4 v4.0.5 // indirect
36+
github.com/godbus/dbus/v5 v5.1.0 // indirect
3337
github.com/gofrs/flock v0.8.1 // indirect
3438
github.com/gofrs/uuid v4.1.0+incompatible // indirect
3539
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
@@ -84,6 +88,7 @@ require (
8488
github.com/ulikunitz/xz v0.5.10 // indirect
8589
github.com/vmihailenco/msgpack/v5 v5.3.5 // indirect
8690
github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect
91+
github.com/zalando/go-keyring v0.2.6 // indirect
8792
go.opencensus.io v0.24.0 // indirect
8893
golang.org/x/exp v0.0.0-20230321023759-10a507213a29 // indirect
8994
golang.org/x/mod v0.26.0 // indirect

‎go.sum‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
al.essio.dev/pkg/shellescape v1.5.1 h1:86HrALUujYS/h+GtqoB26SBEdkWfmMI6FubjXlsXyho=
2+
al.essio.dev/pkg/shellescape v1.5.1/go.mod h1:6sIqp7X2P6mThCQ7twERpZTuigpr6KbZWtls1U8I890=
13
cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
24
cloud.google.com/go v0.110.8 h1:tyNdfIxjzaWctIiLYOTalaLKZ17SI44SKFW26QbOhME=
35
cloud.google.com/go v0.110.8/go.mod h1:Iz8AkXJf1qmxC3Oxoep8R1T36w8B92yU29PcBhHO5fk=
@@ -17,6 +19,8 @@ github.com/ChrisTrenkamp/goxpath v0.0.0-20210404020558-97928f7e12b6/go.mod h1:nu
1719
github.com/DataDog/datadog-go v3.2.0+incompatible/go.mod h1:LButxg5PwREeZtORoXG3tL4fMGNddJ+vMq1mwgfaqoQ=
1820
github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY=
1921
github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU=
22+
github.com/UpCloudLtd/upcloud-go-api/credentials v0.1.1 h1:eTfQsv58ufALOk9BZ7WbS/i7pMUD11RnYYpRPsz0LdI=
23+
github.com/UpCloudLtd/upcloud-go-api/credentials v0.1.1/go.mod h1:7OtVs2UqtfvjkC1HfE+Oud0MnbMv7qUWnbEgxnTAqts=
2024
github.com/UpCloudLtd/upcloud-go-api/v8 v8.25.0 h1:OWkIs5Z67jZJb9qNHNaNCl20vJaCIn9U1QnaRZE5grQ=
2125
github.com/UpCloudLtd/upcloud-go-api/v8 v8.25.0/go.mod h1:ImDdnWfVVM6WCRTrskGhAw2W1cRwu5IEkBw+9UCzAv8=
2226
github.com/agext/levenshtein v1.2.3 h1:YB2fHEn0UJagG8T1rrWknE3ZQzWM06O8AMAatNn7lmo=
@@ -55,6 +59,8 @@ github.com/circonus-labs/circonus-gometrics v2.3.1+incompatible/go.mod h1:nmEj6D
5559
github.com/circonus-labs/circonusllhist v0.1.3/go.mod h1:kMXHVDlOchFAehlya5ePtbp5jckzBHf4XRpQvBOLI+I=
5660
github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
5761
github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc=
62+
github.com/danieljoos/wincred v1.2.2 h1:774zMFJrqaeYCK2W57BgAem/MLi6mtSE47MB6BOJ0i0=
63+
github.com/danieljoos/wincred v1.2.2/go.mod h1:w7w4Utbrz8lqeMbDAK0lkNJUv5sAOkFi7nd/ogr0Uh8=
5864
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
5965
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
6066
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=
@@ -83,6 +89,8 @@ github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V
8389
github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY=
8490
github.com/go-test/deep v1.0.3 h1:ZrJSEWsXzPOxaZnFteGEfooLba+ju3FYIbOrS+rQd68=
8591
github.com/go-test/deep v1.0.3/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3aSFNA=
92+
github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk=
93+
github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
8694
github.com/gofrs/flock v0.8.1 h1:+gYjHKf32LDeiEEFhQaotPbLuUXjY5ZqxKgXy7n59aw=
8795
github.com/gofrs/flock v0.8.1/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU=
8896
github.com/gofrs/uuid v3.2.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM=
@@ -124,6 +132,8 @@ github.com/google/martian/v3 v3.3.2 h1:IqNFLAmvJOgVlpdEBiQbDc2EwKW77amAycfTuWKdf
124132
github.com/google/martian/v3 v3.3.2/go.mod h1:oBOf6HBosgwRXnUGWUB05QECsc6uvmMiJ3+6W4l/CUk=
125133
github.com/google/s2a-go v0.1.7 h1:60BLSyTrOV4/haCDW4zb1guZItoSq8foHCXrAnjBo/o=
126134
github.com/google/s2a-go v0.1.7/go.mod h1:50CgR4k1jNlWBu4UfS4AcfhVe1r6pdZPygJ3R8F0Qdw=
135+
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4=
136+
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ=
127137
github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
128138
github.com/google/uuid v1.4.0 h1:MtMxsa51/r9yyhkyLsVeVt0B+BGQZzpQiTQ4eHZ8bc4=
129139
github.com/google/uuid v1.4.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
@@ -340,6 +350,8 @@ github.com/vmihailenco/msgpack/v5 v5.3.5 h1:5gO0H1iULLWGhs2H5tbAHIZTV8/cYafcFOr9
340350
github.com/vmihailenco/msgpack/v5 v5.3.5/go.mod h1:7xyJ9e+0+9SaZT0Wt1RGleJXzli6Q/V5KbhBonMG9jc=
341351
github.com/vmihailenco/tagparser/v2 v2.0.0 h1:y09buUbR+b5aycVFQs/g70pqKVZNBmxwAhO7/IwNM9g=
342352
github.com/vmihailenco/tagparser/v2 v2.0.0/go.mod h1:Wri+At7QHww0WTrCBeu4J6bNtoV6mEfg5OIWRZA9qds=
353+
github.com/zalando/go-keyring v0.2.6 h1:r7Yc3+H+Ux0+M72zacZoItR3UDxeWfKTcabvkI8ua9s=
354+
github.com/zalando/go-keyring v0.2.6/go.mod h1:2TCrxYrbUNYfNS/Kgy/LSrkSQzZ5UPVH85RwfczwvcI=
343355
github.com/zclconf/go-cty-debug v0.0.0-20240509010212-0d6042c53940 h1:4r45xpDWB6ZMSMNJFMOjqrGHynW3DIBuR2H9j0ug+Mo=
344356
github.com/zclconf/go-cty-debug v0.0.0-20240509010212-0d6042c53940/go.mod h1:CmBdvvj3nqzfzJ6nTCIwDTPZ56aVGvDrmztiO5g3qrM=
345357
go.opencensus.io v0.24.0 h1:y73uSU6J157QMP2kn2r30vwW1A2W2WFwSCGnAVxeaD0=

‎internal/driver/driver.go‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"strings"
1010
"time"
1111

12+
"github.com/UpCloudLtd/upcloud-go-api/credentials"
1213
"github.com/UpCloudLtd/upcloud-go-api/v8/upcloud"
1314
"github.com/UpCloudLtd/upcloud-go-api/v8/upcloud/client"
1415
"github.com/UpCloudLtd/upcloud-go-api/v8/upcloud/request"
@@ -450,22 +451,19 @@ func getNowString() string {
450451
return time.Now().Format("20060102-150405")
451452
}
452453

453-
func UsernameFromEnv() string {
454-
username := os.Getenv(EnvConfigUsernameLegacy)
455-
if username == "" {
456-
username = os.Getenv(EnvConfigUsername)
454+
func CredentialsFromEnv(username, password, token string) (credentials.Credentials, error) {
455+
config := credentials.Credentials{
456+
Username: username,
457+
Password: password,
458+
Token: token,
457459
}
458-
return username
459-
}
460460

461-
func PasswordFromEnv() string {
462-
passwd := os.Getenv(EnvConfigPasswordLegacy)
463-
if passwd == "" {
464-
passwd = os.Getenv(EnvConfigPassword)
461+
if config.Username == "" {
462+
config.Username = os.Getenv(EnvConfigUsernameLegacy)
463+
}
464+
if config.Password == "" {
465+
config.Password = os.Getenv(EnvConfigPasswordLegacy)
465466
}
466-
return passwd
467-
}
468467

469-
func TokenFromEnv() string {
470-
return os.Getenv(EnvConfigAPIToken)
468+
return credentials.Parse(config) //nolint:wrapcheck // Use the original error from shared credentials package
471469
}

0 commit comments

Comments
 (0)