Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix the use of a hardcoded admin user #653

Merged
merged 1 commit into from
Jan 17, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
fix: fix the use of a hardcoded admin user
The terraform provider code used a hardcoded `admin` user in a number of place, which caused issues
if a different username (or client credentails) was used. With this change we use the username (or
clientID) as specified in the provider.

This PR also changes the username that is passed in arguments to SSH key management calls. Juju
3.6 does not associate ssh keys with users - ssh keys are global per model - so the username
is ignored. For consistency we pass in the username that is used to log in to the
controller because in Juju 4 ssh keys will be associated with users.
  • Loading branch information
alesstimec committed Jan 15, 2025
commit 4a5e92ba2be45859d3699b923598253680228e2a
35 changes: 25 additions & 10 deletions internal/juju/client.go
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ import (
"context"
"fmt"
"strconv"
"strings"
"sync"
"time"

@@ -21,15 +22,16 @@ import (
)

const (
PrefixCloud = "cloud-"
PrefixModel = "model-"
PrefixCharm = "charm-"
PrefixUser = "user-"
PrefixMachine = "machine-"
PrefixApplication = "application-"
PrefixStorage = "storage-"
UnspecifiedRevision = -1
connectionTimeout = 30 * time.Second
PrefixCloud = "cloud-"
PrefixModel = "model-"
PrefixCharm = "charm-"
PrefixUser = "user-"
PrefixMachine = "machine-"
PrefixApplication = "application-"
PrefixStorage = "storage-"
UnspecifiedRevision = -1
connectionTimeout = 30 * time.Second
serviceAccountSuffix = "@serviceaccount"
)

type ControllerConfiguration struct {
@@ -54,7 +56,8 @@ type Client struct {
Secrets secretsClient
Jaas jaasClient

isJAAS func() bool
isJAAS func() bool
username string
}

// IsJAAS returns a boolean to indicate whether the controller configured is a JAAS controller.
@@ -63,6 +66,12 @@ func (c Client) IsJAAS() bool {
return c.isJAAS()
}

// Username returns the username specified in the Juju provider or, if specified, the
// service account username.
func (c Client) Username() string {
return c.username
}

type jujuModel struct {
name string
modelType model.ModelType
@@ -104,6 +113,11 @@ func NewClient(ctx context.Context, config ControllerConfiguration) (*Client, er
defaultJAASCheck = true
}

user := config.Username
if config.ClientID != "" && !strings.HasSuffix(config.ClientID, serviceAccountSuffix) {
user = fmt.Sprintf("%s%s", config.ClientID, serviceAccountSuffix)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small thing but in the future we might want to force the user to add the "@serviceaccount" suffix themselves. So you could consider only adding the suffix if it's not already present. Lots of "ifs" so feel free to ignore this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}

return &Client{
Applications: *newApplicationClient(sc),
Clouds: *newKubernetesCloudsClient(sc),
@@ -117,6 +131,7 @@ func NewClient(ctx context.Context, config ControllerConfiguration) (*Client, er
Secrets: *newSecretsClient(sc),
Jaas: *newJaasClient(sc),
isJAAS: func() bool { return sc.IsJAAS(defaultJAASCheck) },
username: user,
}, nil
}

4 changes: 3 additions & 1 deletion internal/juju/offers.go
Original file line number Diff line number Diff line change
@@ -36,6 +36,7 @@ type CreateOfferInput struct {
Endpoint string
ModelName string
ModelOwner string
OfferOwner string
Name string
}

@@ -117,7 +118,8 @@ func (c offersClient) CreateOffer(input *CreateOfferInput) (*CreateOfferResponse
if err != nil {
return nil, append(errs, err)
}
result, err := client.Offer(modelUUID, input.ApplicationName, []string{input.Endpoint}, "admin", offerName, "")

result, err := client.Offer(modelUUID, input.ApplicationName, []string{input.Endpoint}, input.OfferOwner, offerName, "")
if err != nil {
return nil, append(errs, err)
}
22 changes: 13 additions & 9 deletions internal/juju/sshKeys.go
Original file line number Diff line number Diff line change
@@ -17,11 +17,13 @@ type sshKeysClient struct {
}

type CreateSSHKeyInput struct {
Username string
ModelName string
Payload string
}

type ReadSSHKeyInput struct {
Username string
ModelName string
KeyIdentifier string
}
@@ -32,6 +34,7 @@ type ReadSSHKeyOutput struct {
}

type DeleteSSHKeyInput struct {
Username string
ModelName string
KeyIdentifier string
}
@@ -51,9 +54,9 @@ func (c *sshKeysClient) CreateSSHKey(input *CreateSSHKeyInput) error {

client := keymanager.NewClient(conn)

// NOTE
// Juju only stores ssh keys at a global level.
params, err := client.AddKeys("admin", input.Payload)
// NOTE: In Juju 3.6 ssh keys are not associated with user - they are global per model. We pass in
// the logged-in user for completeness. In Juju 4 ssh keys will be associated with users.<
params, err := client.AddKeys(input.Username, input.Payload)
if err != nil {
return err
}
@@ -83,9 +86,9 @@ func (c *sshKeysClient) ReadSSHKey(input *ReadSSHKeyInput) (*ReadSSHKeyOutput, e

client := keymanager.NewClient(conn)

// NOTE: At this moment Juju only uses global ssh keys.
// We hardcode the user to be admin.
returnedKeys, err := client.ListKeys(ssh.FullKeys, "admin")
// NOTE: In Juju 3.6 ssh keys are not associated with user - they are global per model. We pass in
// the logged-in user for completeness. In Juju 4 ssh keys will be associated with users.<
returnedKeys, err := client.ListKeys(ssh.FullKeys, input.Username)
if err != nil {
return nil, err
}
@@ -118,7 +121,7 @@ func (c *sshKeysClient) DeleteSSHKey(input *DeleteSSHKeyInput) error {
// that impacts the current Juju logic. As a temporal workaround
// we will check if this is the latest SSH key of this model and
// skip the delete.
returnedKeys, err := client.ListKeys(ssh.FullKeys, "admin")
returnedKeys, err := client.ListKeys(ssh.FullKeys, input.Username)
if err != nil {
return err
}
@@ -132,8 +135,9 @@ func (c *sshKeysClient) DeleteSSHKey(input *DeleteSSHKeyInput) error {
}
}

// NOTE: Right now Juju uses global users for keys
params, err := client.DeleteKeys("admin", input.KeyIdentifier)
// NOTE: In Juju 3.6 ssh keys are not associated with user - they are global per model. We pass in
// the logged-in user for completeness. In Juju 4 ssh keys will be associated with users.<
params, err := client.DeleteKeys(input.Username, input.KeyIdentifier)
if len(params) != 0 {
messages := make([]string, 0)
for _, e := range params {
1 change: 1 addition & 0 deletions internal/provider/resource_offer.go
Original file line number Diff line number Diff line change
@@ -143,6 +143,7 @@ func (o *offerResource) Create(ctx context.Context, req resource.CreateRequest,
Name: offerName,
ApplicationName: plan.ApplicationName.ValueString(),
Endpoint: plan.EndpointName.ValueString(),
OfferOwner: o.client.Username(),
})
if errs != nil {
// TODO 10-Aug-2023
5 changes: 5 additions & 0 deletions internal/provider/resource_ssh_key.go
Original file line number Diff line number Diff line change
@@ -122,6 +122,7 @@ func (s *sshKeyResource) Create(ctx context.Context, req resource.CreateRequest,
modelName := plan.ModelName.ValueString()

if err := s.client.SSHKeys.CreateSSHKey(&juju.CreateSSHKeyInput{
Username: s.client.Username(),
ModelName: modelName,
Payload: payload,
}); err != nil {
@@ -173,6 +174,7 @@ func (s *sshKeyResource) Read(ctx context.Context, req resource.ReadRequest, res
}

result, err := s.client.SSHKeys.ReadSSHKey(&juju.ReadSSHKeyInput{
Username: s.client.Username(),
ModelName: modelName,
KeyIdentifier: keyIdentifier,
})
@@ -221,6 +223,7 @@ func (s *sshKeyResource) Update(ctx context.Context, req resource.UpdateRequest,

// Delete the key
if err := s.client.SSHKeys.DeleteSSHKey(&juju.DeleteSSHKeyInput{
Username: s.client.Username(),
ModelName: modelName,
KeyIdentifier: keyIdentifier,
}); err != nil {
@@ -231,6 +234,7 @@ func (s *sshKeyResource) Update(ctx context.Context, req resource.UpdateRequest,

// Create a new key
if err := s.client.SSHKeys.CreateSSHKey(&juju.CreateSSHKeyInput{
Username: s.client.Username(),
ModelName: plan.ModelName.ValueString(),
Payload: plan.Payload.ValueString(),
}); err != nil {
@@ -274,6 +278,7 @@ func (s *sshKeyResource) Delete(ctx context.Context, req resource.DeleteRequest,

// Delete the key
if err := s.client.SSHKeys.DeleteSSHKey(&juju.DeleteSSHKeyInput{
Username: s.client.Username(),
ModelName: modelName,
KeyIdentifier: keyIdentifier,
}); err != nil {
Loading