Skip to content

Commit

Permalink
Add support for mapping accounts in authconfigmap
Browse files Browse the repository at this point in the history
  • Loading branch information
rndstr committed Mar 13, 2019
1 parent 5fea1d3 commit fe3620d
Show file tree
Hide file tree
Showing 2 changed files with 172 additions and 31 deletions.
120 changes: 92 additions & 28 deletions pkg/authconfigmap/authconfigmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package authconfigmap

import (
"fmt"
"sort"

"github.com/kris-nova/logger"
"github.com/pkg/errors"
Expand All @@ -26,6 +27,9 @@ const (
objectName = "aws-auth"
objectNamespace = metav1.NamespaceSystem

rolesData = "mapRoles"
accountsData = "mapAccounts"

// GroupMasters is the admin group which is also automatically
// granted to the IAM role that creates the cluster.
GroupMasters = "system:masters"
Expand Down Expand Up @@ -53,9 +57,69 @@ func New(cm *corev1.ConfigMap) *AuthConfigMap {
return a
}

// AddAccount appends an IAM account to the `mapAccounts` entry
// in the configmap. It also deduplicates.
func (a *AuthConfigMap) AddAccount(account string) error {
accounts, err := a.accounts()
if err != nil {
return err
}
distinct := map[string]struct{}{account: {}}
for _, acc := range accounts {
distinct[acc] = struct{}{}
}
accounts = accounts[:0]
for acc := range distinct {
accounts = append(accounts, acc)
}
// List order matters in yamls, maintain deterministic output
sort.Strings(accounts)
return a.setAccounts(accounts)
}

// RemoveAccount removes the given IAM account entry in mapAccounts.
func (a *AuthConfigMap) RemoveAccount(account string) error {
accounts, err := a.accounts()
if err != nil {
return err
}

var newaccounts []string
found := false
for _, acc := range accounts {
if acc == account {
found = true
continue
}
newaccounts = append(newaccounts, acc)
}
if !found {
return fmt.Errorf("account %q not found in config map", account)
}
logger.Info("removing account %s from config map", account)
return a.setAccounts(newaccounts)
}

func (a *AuthConfigMap) accounts() ([]string, error) {
var accounts []string
if err := yaml.Unmarshal([]byte(a.cm.Data[accountsData]), &accounts); err != nil {
return nil, errors.Wrap(err, "unmarshalling mapAccounts")
}
return accounts, nil
}

func (a *AuthConfigMap) setAccounts(accounts []string) error {
bs, err := yaml.Marshal(accounts)
if err != nil {
return errors.Wrap(err, "marshalling mapAccounts")
}
a.cm.Data[accountsData] = string(bs)
return nil
}

// AddRole appends a role with given groups.
func (a *AuthConfigMap) AddRole(arn string, groups []string) error {
roles, err := a.get()
roles, err := a.roles()
if err != nil {
return err
}
Expand All @@ -64,23 +128,45 @@ func (a *AuthConfigMap) AddRole(arn string, groups []string) error {
"username": "system:node:{{EC2PrivateDNSName}}",
"groups": groups,
})
return a.set(roles)
return a.setRoles(roles)
}

func (a *AuthConfigMap) get() (mapRoles, error) {
// RemoveRole removes exactly one entry, even if there are duplicates.
// If it cannot find the role it returns an error.
func (a *AuthConfigMap) RemoveRole(arn string) error {
if arn == "" {
return errors.New("nodegroup instance role ARN is not set")
}
roles, err := a.roles()
if err != nil {
return err
}

for i, role := range roles {
if role["rolearn"] == arn {
logger.Info("removing role %s from config map", arn)
roles = append(roles[:i], roles[i+1:]...)
return a.setRoles(roles)
}
}

return fmt.Errorf("instance role ARN %q not found in config map", arn)
}

func (a *AuthConfigMap) roles() (mapRoles, error) {
var roles mapRoles
if err := yaml.Unmarshal([]byte(a.cm.Data["mapRoles"]), &roles); err != nil {
if err := yaml.Unmarshal([]byte(a.cm.Data[rolesData]), &roles); err != nil {
return nil, errors.Wrap(err, "unmarshalling mapRoles")
}
return roles, nil
}

func (a *AuthConfigMap) set(r mapRoles) error {
func (a *AuthConfigMap) setRoles(r mapRoles) error {
bs, err := yaml.Marshal(r)
if err != nil {
return errors.Wrap(err, "marshalling mapRoles")
}
a.cm.Data["mapRoles"] = string(bs)
a.cm.Data[rolesData] = string(bs)
return nil
}

Expand All @@ -96,28 +182,6 @@ func (a *AuthConfigMap) Save(client v1.ConfigMapInterface) (err error) {
return err
}

// RemoveRole removes exactly one entry, even if there are duplicates.
// If it cannot find the role it returns an error.
func (a *AuthConfigMap) RemoveRole(arn string) error {
if arn == "" {
return errors.New("nodegroup instance role ARN is not set")
}
roles, err := a.get()
if err != nil {
return err
}

for i, role := range roles {
if role["rolearn"] == arn {
logger.Info("removing %s from config map", arn)
roles = append(roles[:i], roles[i+1:]...)
return a.set(roles)
}
}

return fmt.Errorf("instance role ARN %q not found in config map", arn)
}

// ObjectMeta constructs metadata for the configmap
func ObjectMeta() metav1.ObjectMeta {
return metav1.ObjectMeta{
Expand Down
83 changes: 80 additions & 3 deletions pkg/authconfigmap/authconfigmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ import (
)

const (
roleA = "arn:aws:iam::122333:role/eksctl-cluster-5a-nodegroup-ng1-p-NodeInstanceRole-NNH3ISP12CX"
roleB = "arn:aws:iam::122333:role/eksctl-cluster-5a-nodegroup-ng1-p-NodeInstanceRole-ABCDEFGH"
groupB = "foo"
roleA = "arn:aws:iam::122333:role/eksctl-cluster-5a-nodegroup-ng1-p-NodeInstanceRole-NNH3ISP12CX"
roleB = "arn:aws:iam::122333:role/eksctl-cluster-5a-nodegroup-ng1-p-NodeInstanceRole-ABCDEFGH"
groupB = "foo"
accountA = "123"
accountB = "789"
)

var (
Expand All @@ -32,6 +34,17 @@ func makeExpectedRole(arn string, groups []string) string {
`, arn, strings.Join(groups, "\n - "))
}

func makeExpectedAccounts(accounts ...string) string {
var y string
for _, a := range accounts {
// Having them quoted is important for the yaml parser to
// recognize them as strings over numbers
y += fmt.Sprintf("\n- %q", a)
}

return y
}

// mockClient implements v1.ConfigMapInterface
type mockClient struct {
v1.ConfigMapInterface
Expand Down Expand Up @@ -155,4 +168,68 @@ var _ = Describe("AuthConfigMap{}", func() {
Expect(err).To(HaveOccurred())
})
})
Describe("AddAccount()", func() {
cm := &corev1.ConfigMap{
ObjectMeta: ObjectMeta(),
Data: map[string]string{},
}
cm.UID = "123456"
acm := New(cm)

addAndSave := func(account string) *corev1.ConfigMap {
err := acm.AddAccount(account)
Expect(err).NotTo(HaveOccurred())

client := &mockClient{}
err = acm.Save(client)
Expect(err).NotTo(HaveOccurred())
Expect(client.created).To(BeNil())
Expect(client.updated).NotTo(BeNil())

return client.updated
}

It("should add an account", func() {
cm := addAndSave(accountA)
Expect(cm.Data["mapAccounts"]).To(MatchYAML(makeExpectedAccounts(accountA)))
})
It("should deduplicate when adding", func() {
cm := addAndSave(accountA)
Expect(cm.Data["mapAccounts"]).To(MatchYAML(makeExpectedAccounts(accountA)))
})
It("should add another account", func() {
cm := addAndSave(accountB)
Expect(cm.Data["mapAccounts"]).To(MatchYAML(makeExpectedAccounts(accountA) + makeExpectedAccounts(accountB)))
})
})
Describe("RemoveAccount()", func() {
cm := &corev1.ConfigMap{
ObjectMeta: ObjectMeta(),
Data: map[string]string{"mapAccounts": makeExpectedAccounts(accountA) + makeExpectedAccounts(accountB)},
}
cm.UID = "123456"
acm := New(cm)

removeAndSave := func(account string) *corev1.ConfigMap {
err := acm.RemoveAccount(account)
Expect(err).NotTo(HaveOccurred())

client := &mockClient{}
err = acm.Save(client)
Expect(err).NotTo(HaveOccurred())
Expect(client.created).To(BeNil())
Expect(client.updated).NotTo(BeNil())

return client.updated
}

It("should remove an account", func() {
cm := removeAndSave(accountA)
Expect(cm.Data["mapAccounts"]).To(MatchYAML(makeExpectedAccounts(accountB)))
})
It("should fail if account not found", func() {
err := acm.RemoveAccount(accountA)
Expect(err).To(HaveOccurred())
})
})
})

0 comments on commit fe3620d

Please sign in to comment.