Skip to content

Commit

Permalink
Merge pull request juju#17993 from ycliuhw/fix-k8s-secret-backend-config
Browse files Browse the repository at this point in the history
juju#17993

This PR addresses two issues related to the secret backend configuration:

- Juju was incorrectly using the controller's Kubernetes credential for all Kubernetes models, even when the model was associated with a non-controller Kubernetes secret backend (e.g., one added via `add-k8s`).
- The `list-secret-backend` command did not display the correct Kubernetes credentials.

Additionally, the method for fetching the Kubernetes secret backend credential has been moved from the secret backend service, where it used domain cloud and credential state helper functions, to the secret backend state package.

Drive-by: Removed some unused service methods in the secret backend package.

## Checklist

- [x] Code style: imports ordered, good names, simple structure, etc
- [x] Comments saying why design decisions were made
- [x] Go unit tests, with comments saying what you're testing
- [ ] ~[Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing~
- [ ] ~[doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~

## QA steps

```
juju add-secret-backend myvault1 vault --config ./vault.yaml

juju list-secret-backends --reveal --format yaml
internal:
 backend: controller
 secrets: 0
 status: active
myvault1:
 backend: vault
 config:
 endpoint: http://10.245.248.1:8200
 token: root
 secrets: 0
 status: active

juju add-model t1

juju add-secret foo token=1
secret:cr838jvmp25c79sm7m40

juju update-secret foo token=2

juju add-model t2 gke

juju add-secret foo token=1
secret:cr838rfmp25c79sm7m4g

juju list-secret-backends --reveal --format yaml
internal:
 backend: controller
 secrets: 0
 status: active
myvault1:
 backend: vault
 config:
 endpoint: http://10.245.248.1:8200
 token: root
 secrets: 0
 status: active
t1-local:
 backend: kubernetes
 config:
 ca-certs:
 - |
 -----BEGIN CERTIFICATE-----
 ...
 bA0lqx9IWOu1NiegK5PMz7x2SQ==
 -----END CERTIFICATE-----
 credential: '{"auth-type":"oauth2","Attributes":{"Token":"eyJhbGciOiJSUzI1NiIsImtpZCI6Ik...w-m8cdoGDshHTxIKqgtPu8wg"}}'
 endpoint: https://127.0.0.1:16443
 is-controller-cloud: true
 secrets: 2
 status: active
t2-local:
 backend: kubernetes
 config:
 ca-certs:
 - |
 -----BEGIN CERTIFICATE-----
 ...
 X16beckVwn5MA6ngWmUrJnrmO042rKwttaJ8jV22IBV9uC8IzMfL026vUfcdvodH
 EZkziVJR0/uOH+TJqV7eyQ==
 -----END CERTIFICATE-----
 credential: '{"auth-type":"oauth2","Attributes":{"Token":"eyJhbGciOiJSUzI1N...CJMEw","rbac-id":"c38321b5"}}'
 endpoint: https://34.87.147.130
 is-controller-cloud: false
 secrets: 1
 status: active
```

## Documentation changes

No

## Links

**Jira card:** [JUJU-6561](https://warthogs.atlassian.net/browse/JUJU-6561)



[JUJU-6561]: https://warthogs.atlassian.net/browse/JUJU-6561?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
  • Loading branch information
jujubot authored Sep 3, 2024
2 parents bb2c87f + b6279b8 commit 30586b1
Show file tree
Hide file tree
Showing 11 changed files with 503 additions and 818 deletions.
39 changes: 0 additions & 39 deletions apiserver/facades/client/secretbackends/mock_service.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions apiserver/facades/client/secretbackends/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,5 @@ type SecretBackendService interface {
CreateSecretBackend(context.Context, coresecrets.SecretBackend) error
UpdateSecretBackend(context.Context, secretbackendservice.UpdateSecretBackendParams) error
DeleteSecretBackend(context.Context, secretbackendservice.DeleteSecretBackendParams) error
GetSecretBackendByName(context.Context, string) (*coresecrets.SecretBackend, error)

BackendSummaryInfo(ctx context.Context, reveal bool, names ...string) ([]*secretbackendservice.SecretBackendInfo, error)
}
2 changes: 1 addition & 1 deletion domain/secretbackend/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ type SecretBackend struct {
// NextRotateTime is the time at which the next token rotation should occur.
NextRotateTime *time.Time
// Config is the configuration of the secret backend.
Config map[string]string
Config map[string]any
// NumSecrets is the number of secrets stored in the secret backend.
NumSecrets int
}
5 changes: 0 additions & 5 deletions domain/secretbackend/service/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"context"
"time"

"github.com/juju/juju/cloud"
"github.com/juju/juju/core/changestream"
coremodel "github.com/juju/juju/core/model"
"github.com/juju/juju/core/watcher"
Expand All @@ -17,16 +16,12 @@ import (

// State provides methods for working with secret backends.
type State interface {
GetControllerModelCloudAndCredential(ctx context.Context) (cloud.Cloud, cloud.Credential, error)
GetModelCloudAndCredential(ctx context.Context, modelUUID coremodel.UUID) (cloud.Cloud, cloud.Credential, error)

CreateSecretBackend(ctx context.Context, params secretbackend.CreateSecretBackendParams) (string, error)
UpdateSecretBackend(ctx context.Context, params secretbackend.UpdateSecretBackendParams) (string, error)
DeleteSecretBackend(ctx context.Context, _ secretbackend.BackendIdentifier, deleteInUse bool) error
ListSecretBackends(ctx context.Context) ([]*secretbackend.SecretBackend, error)
ListSecretBackendIDs(ctx context.Context) ([]string, error)
ListSecretBackendsForModel(ctx context.Context, modelUUID coremodel.UUID, includeEmpty bool) ([]*secretbackend.SecretBackend, error)
ListInUseKubernetesSecretBackends(ctx context.Context) ([]*secretbackend.SecretBackend, error)
GetSecretBackend(context.Context, secretbackend.BackendIdentifier) (*secretbackend.SecretBackend, error)
SecretBackendRotated(ctx context.Context, backendID string, next time.Time) error

Expand Down
141 changes: 15 additions & 126 deletions domain/secretbackend/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"github.com/juju/errors"
"github.com/juju/names/v5"

"github.com/juju/juju/cloud"
"github.com/juju/juju/core/changestream"
"github.com/juju/juju/core/leadership"
"github.com/juju/juju/core/logger"
Expand All @@ -26,7 +25,6 @@ import (
secretservice "github.com/juju/juju/domain/secret/service"
"github.com/juju/juju/domain/secretbackend"
secretbackenderrors "github.com/juju/juju/domain/secretbackend/errors"
"github.com/juju/juju/environs/cloudspec"
internalsecrets "github.com/juju/juju/internal/secrets"
"github.com/juju/juju/internal/secrets/provider"
"github.com/juju/juju/internal/secrets/provider/juju"
Expand Down Expand Up @@ -65,55 +63,41 @@ func newService(
}
}

// For testing.
var (
GetProvider = provider.Provider
)

// GetSecretBackendConfigForAdmin returns the secret backend configuration for the given backend ID for an admin user.
// GetSecretBackendConfigForAdmin returns the secret backend configuration for the given backend ID for an admin user,
// returning an error satisfying [secretbackenderrors.NotFound] if the backend is not found.
func (s *Service) GetSecretBackendConfigForAdmin(ctx context.Context, modelUUID coremodel.UUID) (*provider.ModelBackendConfigInfo, error) {
m, err := s.st.GetModelSecretBackendDetails(ctx, modelUUID)
if err != nil {
return nil, errors.Trace(err)
}
currentBackend, err := s.st.GetSecretBackend(ctx, secretbackend.BackendIdentifier{ID: m.SecretBackendID})
if err != nil {
return nil, errors.Trace(err)
}
currentBackendName := m.SecretBackendName

var info provider.ModelBackendConfigInfo
info.Configs = make(map[string]provider.ModelBackendConfig)

// We need to include builtin backends for secret draining and accessing those secrets while drain is in progress.
// TODO(secrets) - only use those in use by model
// For now, we'll return all backends on the controller.
backends, err := s.st.ListSecretBackendsForModel(ctx, modelUUID, true)
if err != nil {
return nil, errors.Trace(err)
}
for _, b := range backends {
if b.Name == currentBackend.Name {
if b.Name == currentBackendName {
info.ActiveID = b.ID
}

cfg := convertConfigToAny(b.Config)
if b.Name == kubernetes.BackendName {
var err error
if cfg, err = s.tryControllerModelK8sBackendConfig(ctx); err != nil {
return nil, errors.Trace(err)
}
}
info.Configs[b.ID] = provider.ModelBackendConfig{
ControllerUUID: m.ControllerUUID,
ModelUUID: m.ModelID.String(),
ModelName: m.ModelName,
BackendConfig: provider.BackendConfig{
BackendType: b.BackendType,
Config: cfg,
Config: b.Config,
},
}
}
if info.ActiveID == "" {
return nil, fmt.Errorf("%w: %q", secretbackenderrors.NotFound, currentBackend.Name)
return nil, fmt.Errorf("%w: %q", secretbackenderrors.NotFound, currentBackendName)
}
return &info, nil
}
Expand Down Expand Up @@ -202,7 +186,7 @@ func (s *Service) backendConfigInfo(
return nil, errors.Errorf("unexpected nil value for GrantedSecretsGetter")
}

p, err := GetProvider(adminCfg.BackendType)
p, err := s.registry(adminCfg.BackendType)
if err != nil {
return nil, errors.Trace(err)
}
Expand Down Expand Up @@ -310,15 +294,6 @@ func (s *Service) backendConfigInfo(
return info, nil
}

func convertConfigToAny(config map[string]string) map[string]interface{} {
if len(config) == 0 {
return nil
}
return transform.Map(config, func(k string, v string) (string, any) {
return k, v
})
}

func convertConfigToString(config map[string]interface{}) map[string]string {
if len(config) == 0 {
return nil
Expand All @@ -328,31 +303,6 @@ func convertConfigToString(config map[string]interface{}) map[string]string {
})
}

func getK8sBackendConfig(cloud cloud.Cloud, cred cloud.Credential) (*provider.BackendConfig, error) {
spec, err := cloudspec.MakeCloudSpec(cloud, "", &cred)
if err != nil {
return nil, errors.Trace(err)
}
k8sConfig, err := kubernetes.BuiltInConfig(spec)
if err != nil {
return nil, errors.Trace(err)
}
return k8sConfig, nil
}

// tryControllerModelK8sBackendConfig returns the k8s backend info for the controller model UUID if it's possible.
func (s *Service) tryControllerModelK8sBackendConfig(ctx context.Context) (provider.ConfigAttrs, error) {
cloud, cred, err := s.st.GetControllerModelCloudAndCredential(ctx)
if err != nil {
return nil, errors.Trace(err)
}
k8sConfig, err := getK8sBackendConfig(cloud, cred)
if err != nil {
return nil, errors.Trace(err)
}
return k8sConfig.Config, nil
}

// BackendSummaryInfoForModel returns a summary of the secret backends
// which contain secrets from the specified model.
func (s *Service) BackendSummaryInfoForModel(ctx context.Context, modelUUID coremodel.UUID) ([]*SecretBackendInfo, error) {
Expand All @@ -368,12 +318,12 @@ func (s *Service) BackendSummaryInfoForModel(ctx context.Context, modelUUID core
Name: b.Name,
BackendType: b.BackendType,
TokenRotateInterval: b.TokenRotateInterval,
Config: convertConfigToAny(b.Config),
Config: b.Config,
},
NumSecrets: b.NumSecrets,
})
}
return s.composeBackendInfoResults(ctx, backendInfos, false)
return s.composeBackendInfoResults(backendInfos, false)
}

// ListBackendIDs returns the IDs of all the secret backends.
Expand All @@ -394,48 +344,26 @@ func (s *Service) BackendSummaryInfo(ctx context.Context, reveal bool, names ...
}
backendInfos := make([]*SecretBackendInfo, 0, len(backends))
for _, b := range backends {
if b.Name == kubernetes.BackendName {
// We only care about non kubernetes backend here and
// the kubernetes backend info will be fetched later using the reference count data.
continue
}
backendInfos = append(backendInfos, &SecretBackendInfo{
SecretBackend: coresecrets.SecretBackend{
ID: b.ID,
Name: b.Name,
BackendType: b.BackendType,
TokenRotateInterval: b.TokenRotateInterval,
Config: convertConfigToAny(b.Config),
},
NumSecrets: b.NumSecrets,
})
}

k8sBackends, err := s.st.ListInUseKubernetesSecretBackends(ctx)
if err != nil {
return nil, errors.Trace(err)
}
for _, b := range k8sBackends {
backendInfos = append(backendInfos, &SecretBackendInfo{
SecretBackend: coresecrets.SecretBackend{
ID: b.ID,
Name: b.Name,
BackendType: b.BackendType,
// TODO: fetch the correct config for non controller model k8s backend.
// https://warthogs.atlassian.net/browse/JUJU-6561
Config: b.Config,
},
NumSecrets: b.NumSecrets,
})
}

results, err := s.composeBackendInfoResults(ctx, backendInfos, reveal, names...)
results, err := s.composeBackendInfoResults(backendInfos, reveal, names...)
if err != nil {
return nil, errors.Trace(err)
}
return results, nil
}

func (s *Service) composeBackendInfoResults(ctx context.Context, backendInfos []*SecretBackendInfo, reveal bool, names ...string) ([]*SecretBackendInfo, error) {
func (s *Service) composeBackendInfoResults(backendInfos []*SecretBackendInfo, reveal bool, names ...string) ([]*SecretBackendInfo, error) {
wanted := set.NewStrings(names...)
for i := 0; i < len(backendInfos); {
b := backendInfos[i]
Expand Down Expand Up @@ -473,23 +401,6 @@ func (s *Service) composeBackendInfoResults(ctx context.Context, backendInfos []
return backendInfos, nil
}

// PingSecretBackend checks the secret backend for the given backend name.
func (s *Service) PingSecretBackend(ctx context.Context, name string) error {
backend, err := s.st.GetSecretBackend(ctx, secretbackend.BackendIdentifier{Name: name})
if err != nil {
return errors.Trace(err)
}
p, err := s.registry(backend.BackendType)
if err != nil {
return errors.Trace(err)
}
err = pingBackend(p, convertConfigToAny(backend.Config))
if err != nil {
return fmt.Errorf("cannot ping secret backend %q: %w", name, err)
}
return nil
}

// pingBackend instantiates a backend and pings it.
func pingBackend(p provider.SecretBackendProvider, cfg provider.ConfigAttrs) error {
b, err := p.NewBackend(&provider.ModelBackendConfig{
Expand Down Expand Up @@ -613,7 +524,7 @@ func (s *Service) UpdateSecretBackend(ctx context.Context, params UpdateSecretBa
cfgToApply[k] = defaultVal
}
}
err = configValidator.ValidateConfig(convertConfigToAny(existing.Config), cfgToApply)
err = configValidator.ValidateConfig(existing.Config, cfgToApply)
if err != nil {
return fmt.Errorf("%w: config for provider %q: %w", secretbackenderrors.NotValid, existing.BackendType, err)
}
Expand Down Expand Up @@ -643,28 +554,6 @@ func (s *Service) DeleteSecretBackend(ctx context.Context, params DeleteSecretBa
return s.st.DeleteSecretBackend(ctx, params.BackendIdentifier, params.DeleteInUse)
}

// GetSecretBackendByName returns the secret backend for the given backend name.
func (s *Service) GetSecretBackendByName(ctx context.Context, name string) (*coresecrets.SecretBackend, error) {
sb, err := s.st.GetSecretBackend(ctx, secretbackend.BackendIdentifier{Name: name})
if err != nil {
return nil, errors.Trace(err)
}
cfg := convertConfigToAny(sb.Config)
if name == kubernetes.BackendName {
var err error
if cfg, err = s.tryControllerModelK8sBackendConfig(ctx); err != nil {
return nil, errors.Trace(err)
}
}
return &coresecrets.SecretBackend{
ID: sb.ID,
Name: sb.Name,
BackendType: sb.BackendType,
TokenRotateInterval: sb.TokenRotateInterval,
Config: cfg,
}, nil
}

// RotateBackendToken rotates the token for the given secret backend.
func (s *Service) RotateBackendToken(ctx context.Context, backendID string) error {
backendInfo, err := s.st.GetSecretBackend(ctx,
Expand All @@ -689,7 +578,7 @@ func (s *Service) RotateBackendToken(ctx context.Context, backendID string) erro
s.logger.Debugf("refresh token for backend %v", backendInfo.Name)
cfg := provider.BackendConfig{
BackendType: backendInfo.BackendType,
Config: convertConfigToAny(backendInfo.Config),
Config: backendInfo.Config,
}
// Ideally, we should do this in a transaction, but it's not critical.
// Because it's called by a single worker at a time.
Expand Down
Loading

0 comments on commit 30586b1

Please sign in to comment.