Skip to content

Commit bbf3e8f

Browse files
artem-lifshitsartem-lifshits
authored
Cloud: fix token after rotation (#141)
Cloud: fix token after rotation Remove old client after cloud rotation Acceptance tests vault-plugin-secrets-openstack> go test -count=1 -v -timeout=20m -tags=acceptance ./acceptance === RUN TestPlugin === RUN TestPlugin/TestCloudLifecycle === RUN TestPlugin/TestCloudLifecycle/WriteCloud === RUN TestPlugin/TestCloudLifecycle/ReadCloud === RUN TestPlugin/TestCloudLifecycle/ListClouds === RUN TestPlugin/TestCloudLifecycle/ListClouds/method-LIST === PAUSE TestPlugin/TestCloudLifecycle/ListClouds/method-LIST === RUN TestPlugin/TestCloudLifecycle/ListClouds/method-GET === PAUSE TestPlugin/TestCloudLifecycle/ListClouds/method-GET === CONT TestPlugin/TestCloudLifecycle/ListClouds/method-LIST === CONT TestPlugin/TestCloudLifecycle/ListClouds/method-GET === RUN TestPlugin/TestCloudLifecycle/DeleteCloud === RUN TestPlugin/TestCredsLifecycle === RUN TestPlugin/TestCredsLifecycle/root_token === RUN TestPlugin/TestCredsLifecycle/user_token creds_test.go:98: Skipping test that requires admin privileges (set OS_TEST_ADMIN=1 to run) === RUN TestPlugin/TestCredsLifecycle/user_password creds_test.go:98: Skipping test that requires admin privileges (set OS_TEST_ADMIN=1 to run) === RUN TestPlugin/TestCredsLifecycle/user_domain_id_token creds_test.go:98: Skipping test that requires admin privileges (set OS_TEST_ADMIN=1 to run) === RUN TestPlugin/TestInfo === RUN TestPlugin/TestRoleLifecycle === RUN TestPlugin/TestRoleLifecycle/WriteRole === RUN TestPlugin/TestRoleLifecycle/ReadRole === RUN TestPlugin/TestRoleLifecycle/ListRoles === RUN TestPlugin/TestRoleLifecycle/ListRoles/method-LIST === PAUSE TestPlugin/TestRoleLifecycle/ListRoles/method-LIST === RUN TestPlugin/TestRoleLifecycle/ListRoles/method-GET === PAUSE TestPlugin/TestRoleLifecycle/ListRoles/method-GET === CONT TestPlugin/TestRoleLifecycle/ListRoles/method-LIST === CONT TestPlugin/TestRoleLifecycle/ListRoles/method-GET === RUN TestPlugin/TestRoleLifecycle/DeleteRole === RUN TestPlugin/TestRootRotate rotate_test.go:65: Skipping test that requires admin privileges (set OS_TEST_ADMIN=1 to run) === RUN TestPlugin/TestStaticCredsLifecycle static_creds_test.go:35: Skipping test that requires admin privileges (set OS_TEST_ADMIN=1 to run) === RUN TestPlugin/TestStaticRoleLifecycle static_roles_test.go:47: Skipping test that requires admin privileges (set OS_TEST_ADMIN=1 to run) --- PASS: TestPlugin (2.14s) --- PASS: TestPlugin/TestCloudLifecycle (0.06s) --- PASS: TestPlugin/TestCloudLifecycle/WriteCloud (0.05s) --- PASS: TestPlugin/TestCloudLifecycle/ReadCloud (0.00s) --- PASS: TestPlugin/TestCloudLifecycle/ListClouds (0.00s) --- PASS: TestPlugin/TestCloudLifecycle/ListClouds/method-LIST (0.00s) --- PASS: TestPlugin/TestCloudLifecycle/ListClouds/method-GET (0.01s) --- PASS: TestPlugin/TestCloudLifecycle/DeleteCloud (0.00s) --- PASS: TestPlugin/TestCredsLifecycle (1.31s) --- PASS: TestPlugin/TestCredsLifecycle/root_token (0.75s) --- SKIP: TestPlugin/TestCredsLifecycle/user_token (0.00s) --- SKIP: TestPlugin/TestCredsLifecycle/user_password (0.00s) --- SKIP: TestPlugin/TestCredsLifecycle/user_domain_id_token (0.00s) --- PASS: TestPlugin/TestInfo (0.00s) --- PASS: TestPlugin/TestRoleLifecycle (0.46s) --- PASS: TestPlugin/TestRoleLifecycle/WriteRole (0.46s) --- PASS: TestPlugin/TestRoleLifecycle/ReadRole (0.00s) --- PASS: TestPlugin/TestRoleLifecycle/ListRoles (0.00s) --- PASS: TestPlugin/TestRoleLifecycle/ListRoles/method-LIST (0.00s) --- PASS: TestPlugin/TestRoleLifecycle/ListRoles/method-GET (0.00s) --- PASS: TestPlugin/TestRoleLifecycle/DeleteRole (0.00s) --- SKIP: TestPlugin/TestRootRotate (0.00s) --- SKIP: TestPlugin/TestStaticCredsLifecycle (0.00s) --- SKIP: TestPlugin/TestStaticRoleLifecycle (0.00s) PASS ok github.com/opentelekomcloud/vault-plugin-secrets-openstack/acceptance 2.296s Unit tests PS C:\Users\li-ar\GolandProjects\vault-plugin-secrets-openstack> go test ./openstack/... -count=1 ok github.com/opentelekomcloud/vault-plugin-secrets-openstack/openstack 0.212s ? github.com/opentelekomcloud/vault-plugin-secrets-openstack/openstack/common [no test files] ? github.com/opentelekomcloud/vault-plugin-secrets-openstack/openstack/fixtures [no test files] Reviewed-by: Anton Sidelnikov
1 parent a4eb56e commit bbf3e8f

File tree

11 files changed

+76
-18
lines changed

11 files changed

+76
-18
lines changed

‎.golangci.yaml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1+
version: "2"
2+
13
run:
24
modules-download-mode: readonly
35

46
issues:
5-
max-per-linter: 0
67
max-same-issues: 0

‎acceptance/creds_test.go‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package acceptance
66
import (
77
"fmt"
88
"net/http"
9+
"os"
910
"testing"
1011

1112
"github.com/opentelekomcloud/vault-plugin-secrets-openstack/openstack"
@@ -35,6 +36,9 @@ func (p *PluginTest) TestCredsLifecycle() {
3536

3637
_, aux := openstackClient(t)
3738

39+
testGroup := os.Getenv("OS_TEST_GROUP")
40+
testRole := os.Getenv("OS_TEST_ROLE")
41+
3842
cases := map[string]testCase{
3943
"root_token": {
4044
Cloud: cloud.Name,
@@ -48,7 +52,6 @@ func (p *PluginTest) TestCredsLifecycle() {
4852
DomainID: aux.DomainID,
4953
Root: false,
5054
SecretType: "token",
51-
UserGroups: []string{"mygroup"},
5255
Extensions: map[string]interface{}{
5356
"identity_api_version": "3",
5457
},
@@ -69,17 +72,32 @@ func (p *PluginTest) TestCredsLifecycle() {
6972
UserDomainID: aux.DomainID,
7073
Root: false,
7174
SecretType: "token",
72-
UserRoles: []string{"member"},
7375
Extensions: map[string]interface{}{
7476
"identity_api_version": "3",
7577
},
7678
},
7779
}
7880

81+
if testGroup != "" {
82+
tc := cases["user_token"]
83+
tc.UserGroups = []string{testGroup}
84+
cases["user_token"] = tc
85+
}
86+
87+
if testRole != "" {
88+
tc := cases["user_domain_id_token"]
89+
tc.UserRoles = []string{testRole}
90+
cases["user_domain_id_token"] = tc
91+
}
92+
7993
for name, data := range cases {
8094
t.Run(name, func(t *testing.T) {
8195
data := data
8296

97+
if !data.Root && os.Getenv("OS_TEST_ADMIN") == "" {
98+
t.Skip("Skipping test that requires admin privileges (set OS_TEST_ADMIN=1 to run)")
99+
}
100+
83101
_, err := p.vaultDo(
84102
http.MethodPost,
85103
pluginPwdPolicyEndpoint,

‎acceptance/info_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,5 +39,6 @@ func (p *PluginTest) TestInfo() {
3939
require.Equal(t, http.StatusOK, resp.StatusCode)
4040

4141
data := extractInfoData(t, resp)
42-
assert.NotEmpty(t, data)
42+
// Note: version info fields may be empty if built without ldflags
43+
assert.NotNil(t, data)
4344
}

‎acceptance/rotate_test.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package acceptance
66
import (
77
"fmt"
88
"net/http"
9+
"os"
910

1011
"github.com/gophercloud/gophercloud/openstack/identity/v3/roles"
1112
"github.com/gophercloud/gophercloud/openstack/identity/v3/users"
@@ -60,6 +61,10 @@ func (p *PluginTest) makeChildCloud(base *openstack.OsCloud) *openstack.OsCloud
6061
func (p *PluginTest) TestRootRotate() {
6162
t := p.T()
6263

64+
if os.Getenv("OS_TEST_ADMIN") == "" {
65+
t.Skip("Skipping test that requires admin privileges (set OS_TEST_ADMIN=1 to run)")
66+
}
67+
6368
cloud := openstackCloudConfig(t)
6469
require.NotEmpty(t, cloud)
6570
p.makeCloud(cloud)

‎acceptance/static_creds_test.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,16 @@ package acceptance
55

66
import (
77
"fmt"
8+
"net/http"
9+
"os"
10+
"testing"
11+
812
"github.com/gophercloud/gophercloud"
913
"github.com/gophercloud/gophercloud/openstack/identity/v3/roles"
1014
"github.com/gophercloud/gophercloud/openstack/identity/v3/users"
1115
"github.com/opentelekomcloud/vault-plugin-secrets-openstack/openstack"
1216
"github.com/opentelekomcloud/vault-plugin-secrets-openstack/openstack/fixtures"
1317
"github.com/stretchr/testify/require"
14-
"net/http"
15-
"testing"
1618
)
1719

1820
type testStaticCase struct {
@@ -29,6 +31,10 @@ type testStaticCase struct {
2931
func (p *PluginTest) TestStaticCredsLifecycle() {
3032
t := p.T()
3133

34+
if os.Getenv("OS_TEST_ADMIN") == "" {
35+
t.Skip("Skipping test that requires admin privileges (set OS_TEST_ADMIN=1 to run)")
36+
}
37+
3238
cloud := openstackCloudConfig(t)
3339
require.NotEmpty(t, cloud)
3440

‎acceptance/static_roles_test.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package acceptance
66
import (
77
"fmt"
88
"net/http"
9+
"os"
910
"testing"
1011
"time"
1112

@@ -42,6 +43,10 @@ func extractStaticRoleData(t *testing.T, resp *http.Response) *staticRoleData {
4243
func (p *PluginTest) TestStaticRoleLifecycle() {
4344
t := p.T()
4445

46+
if os.Getenv("OS_TEST_ADMIN") == "" {
47+
t.Skip("Skipping test that requires admin privileges (set OS_TEST_ADMIN=1 to run)")
48+
}
49+
4550
cloud := openstackCloudConfig(t)
4651
require.NotEmpty(t, cloud)
4752

‎openstack/backend.go‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,14 @@ package openstack
33
import (
44
"context"
55
"fmt"
6-
"github.com/gophercloud/gophercloud/openstack/identity/v3/users"
7-
"github.com/hashicorp/go-multierror"
8-
"github.com/opentelekomcloud/vault-plugin-secrets-openstack/openstack/common"
96
"net/http"
107
"sync"
118
"time"
129

10+
"github.com/gophercloud/gophercloud/openstack/identity/v3/users"
11+
"github.com/hashicorp/go-multierror"
12+
"github.com/opentelekomcloud/vault-plugin-secrets-openstack/openstack/common"
13+
1314
"github.com/gophercloud/gophercloud"
1415
"github.com/gophercloud/gophercloud/openstack"
1516
"github.com/gophercloud/gophercloud/openstack/identity/v3/tokens"
@@ -36,6 +37,7 @@ type sharedCloud struct {
3637
type backend struct {
3738
*framework.Backend
3839
clouds map[string]*sharedCloud
40+
cloudsLock sync.RWMutex
3941
checkAutoRotateAfter time.Time
4042
}
4143

@@ -77,6 +79,9 @@ func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend,
7779
}
7880

7981
func (b *backend) getSharedCloud(name string) *sharedCloud {
82+
b.cloudsLock.Lock()
83+
defer b.cloudsLock.Unlock()
84+
8085
passwords := &Passwords{PolicyGenerator: b.System()}
8186
if c, ok := b.clouds[name]; ok {
8287
if c.passwords == nil {
@@ -194,6 +199,9 @@ func (b *backend) rotateIfRequired(ctx context.Context, req *logical.Request, sC
194199
if err != nil {
195200
return err
196201
}
202+
if cloudConfig == nil {
203+
return nil
204+
}
197205
if time.Now().After(cloudConfig.RootPasswordExpirationDate) {
198206
client, err := sCloud.getClient(ctx, req.Storage)
199207
if err != nil {
@@ -226,6 +234,10 @@ func (b *backend) rotateIfRequired(ctx context.Context, req *logical.Request, sC
226234
if err := cloudConfig.save(ctx, req.Storage); err != nil {
227235
return err
228236
}
237+
238+
// Reset cached client to force re-authentication with new password
239+
sCloud.client = nil
240+
229241
b.Logger().Debug("password rotated", "cloud", cloudConfig.Name)
230242
}
231243
return nil

‎openstack/backend_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,18 @@ package openstack
33
import (
44
"context"
55
"fmt"
6-
"github.com/hashicorp/vault/sdk/helper/logging"
76
"net/http"
87
"sync"
98
"testing"
109
"time"
1110

11+
"github.com/hashicorp/vault/sdk/helper/logging"
12+
1213
"github.com/gophercloud/gophercloud"
1314
"github.com/gophercloud/gophercloud/acceptance/tools"
1415
th "github.com/gophercloud/gophercloud/testhelper"
1516
thClient "github.com/gophercloud/gophercloud/testhelper/client"
1617
"github.com/hashicorp/go-hclog"
17-
log "github.com/hashicorp/go-hclog"
1818
"github.com/hashicorp/vault/sdk/framework"
1919
"github.com/hashicorp/vault/sdk/logical"
2020
"github.com/stretchr/testify/assert"
@@ -177,7 +177,7 @@ func TestPeriodicFuncNilConfig(t *testing.T) {
177177
b, _ := testBackend(t)
178178

179179
config := &logical.BackendConfig{
180-
Logger: logging.NewVaultLogger(log.Trace),
180+
Logger: logging.NewVaultLogger(hclog.Trace),
181181
System: &logical.StaticSystemView{
182182
DefaultLeaseTTLVal: defaultLeaseTTLHr,
183183
MaxLeaseTTLVal: maxLeaseTTLHr,

‎openstack/path_rotate_root.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package openstack
33
import (
44
"context"
55
"fmt"
6+
67
"github.com/opentelekomcloud/vault-plugin-secrets-openstack/openstack/common"
78
"github.com/opentelekomcloud/vault-plugin-secrets-openstack/vars"
89

@@ -67,6 +68,9 @@ func (b *backend) rotateRootCredentials(ctx context.Context, req *logical.Reques
6768
if err != nil {
6869
return nil, fmt.Errorf(vars.ErrCloudConf)
6970
}
71+
if cloudConfig == nil {
72+
return nil, fmt.Errorf("cloud %q not found", cloudName)
73+
}
7074

7175
newPassword, err := sharedCloud.passwords.Generate(ctx)
7276
if err != nil {
@@ -91,5 +95,8 @@ func (b *backend) rotateRootCredentials(ctx context.Context, req *logical.Reques
9195
return nil, err
9296
}
9397

98+
// Reset cached client to force re-authentication with new password
99+
sharedCloud.client = nil
100+
94101
return &logical.Response{}, nil
95102
}

‎openstack/path_static_creds.go‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,18 @@ func (b *backend) pathStaticCredsRead(ctx context.Context, r *logical.Request, d
7878
if err != nil {
7979
return nil, err
8080
}
81+
if role == nil {
82+
return nil, fmt.Errorf("static role %q not found", roleName)
83+
}
8184

8285
sharedCloud := b.getSharedCloud(role.Cloud)
8386
cloudConfig, err := sharedCloud.getCloudConfig(ctx, r.Storage)
8487
if err != nil {
8588
return nil, fmt.Errorf(vars.ErrCloudConf)
8689
}
90+
if cloudConfig == nil {
91+
return nil, fmt.Errorf("cloud %q not found", role.Cloud)
92+
}
8793

8894
client, err := sharedCloud.getClient(ctx, r.Storage)
8995
if err != nil {
@@ -158,12 +164,11 @@ func (b *backend) rotateStaticCreds(ctx context.Context, r *logical.Request, d *
158164
if err != nil {
159165
return nil, err
160166
}
161-
162-
sharedCloud := b.getSharedCloud(role.Cloud)
163-
if err != nil {
164-
return nil, err
167+
if role == nil {
168+
return nil, fmt.Errorf("static role %q not found", roleName)
165169
}
166170

171+
sharedCloud := b.getSharedCloud(role.Cloud)
167172
client, err := sharedCloud.getClient(ctx, r.Storage)
168173
if err != nil {
169174
return nil, logical.CodedError(http.StatusConflict, common.LogHttpError(err).Error())

0 commit comments

Comments
 (0)