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 CRD mapper blocking all others because caches never sync and revamp backend-mode flag #303

Merged
merged 6 commits into from
Apr 7, 2020
Merged
Show file tree
Hide file tree
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
62 changes: 55 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,17 +82,54 @@ You may also need to restart the kubelet daemon on your master node to pick up t
systemctl restart kubelet.service
```

### Configure IAMIdentityMapping Custom Resource Definitions

In the `master` version of the AWS IAM Authenticator you can configure your users using one of three methods. The `mapUsers` and `mapRoles` as seen in the [Full Configuration Format](#full-configuration-format), using the new (alpha) [Kubernetes Custom Resource Definitions](https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/), or using an [EKS aws-auth ConfigMap](https://docs.aws.amazon.com/eks/latest/userguide/add-user-role.html). The `--backend-mode` flag determines which of these methods is enabled and their order of precedence (first match wins). The CRD and ConfigMap allow the authenticator server to update its IAM identity mappings without restarting. See [Issues #79](https://github.com/kubernetes-sigs/aws-iam-authenticator/issues/79) for more details.

To setup an `IAMIdentityMapping` CRD you'll first need to `apply` the CRD manifest:
### 4. Create IAM role/user to kubernetes user/group mappings
The default behavior of the server is to source mappings exclusively from the
`mapUsers` and `mapRoles` fields of its configuration file. See [Full
Configuration Format](#full-configuration-format) below for details.

Using the `--backend-mode` flag, you can configure the server to source
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice Job on Improving the documentation and clarification 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, this explanation is helpful.

mappings from two additional backends: an EKS-style ConfigMap
(`--backend-mode=EKSConfigMap`) or `IAMIdentityMapping` custom resources
(`--backend-mode=CRD`). The default backend, the server configuration file
that's mounted by the server pod, corresponds to `--backend-mode=MountedFile`.

You can pass a comma-separated list of these backends to have the server search
them in order. For example, with `--backend-mode=EKSConfigMap,MountedFile`, the
server will search the EKS-style ConfigMap for mappings then, if it doesn't
find a mapping for the given IAM role/user, the server configuration file. If a
mapping for the same IAM role/user exists in multiple backends, the server will
use the mapping in the backend that occurs first in the comma-separated list.
In this example, if a mapping is found in the EKS ConfigMap then it will be
used whether or not a duplicate or conflicting mapping exists in the server
configuration file.

Note that when setting a single backend, the server will *only* source from
that one and ignore the others even if they exist. For example, with
`--backend-mode=CRD`, the server will *only* source from `IAMIdentityMappings`
and ignore the mounted file and EKS ConfigMap.

#### `MountedFile`
This is the default backend of mappings and sufficient for most users. See
[Full Configuration Format](#full-configuration-format) below for details.

#### `CRD` (alpha)
This backend models each IAM mapping as an `IAMIdentityMapping` [Kubernetes
Custom
Resource](https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/).
This approach enables you to maintain mappings in a Kubernetes-native way using
kubectl or the API. Plus, syntax errors (like misaligned YAML) can be more
easily caught and won't affect all mappings.

To setup an `IAMIdentityMapping` CRD you'll first need to `apply` the CRD
manifest:

```
kubectl apply -f deploy/iamidentitymapping.yaml
```

With the CRDs deployed you can then create Custom Resources which model your IAM Identities see [`./deploy/example-iamidentitymapping.yaml`](deploy/example-iamidentitymapping.yaml) :
With the CRDs deployed you can then create Custom Resources which model your
wongma7 marked this conversation as resolved.
Show resolved Hide resolved
IAM Identities. See
[`./deploy/example-iamidentitymapping.yaml`](deploy/example-iamidentitymapping.yaml):

```
---
Expand All @@ -113,7 +150,15 @@ spec:
- system:masters
```

### 4. Set up kubectl to use authentication tokens provided by AWS IAM Authenticator for Kubernetes
#### `EKSConfigMap`
The EKS-style `kube-system/aws-auth` ConfigMap serves as the backend. The
ConfigMap is expected to be in exactly the same format as in EKS clusters:
https://docs.aws.amazon.com/eks/latest/userguide/add-user-role.html. This is
useful if you're migrating from/to EKS and want to keep your mappings, or are
running EKS in addition to some other AWS cluster(s) and want to have the same
mappings in each.

### 5. Set up kubectl to use authentication tokens provided by AWS IAM Authenticator for Kubernetes

> This requires a 1.10+ `kubectl` binary to work. If you receive `Please enter Username:` when trying to use `kubectl` you need to update to the latest `kubectl`

Expand Down Expand Up @@ -407,6 +452,9 @@ server:
- "012345678901"
- "456789012345"

# source mappings from this file (mapUsers, mapRoles, & mapAccounts)
backendMode:
- MountedFile
```

## Community, discussion, contribution, and support
Expand Down
20 changes: 1 addition & 19 deletions cmd/aws-iam-authenticator/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/spf13/cobra"
"github.com/spf13/viper"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/component-base/featuregate"
)

Expand Down Expand Up @@ -113,7 +112,7 @@ func getConfig() (config.Config, error) {
return cfg, errors.New("cluster ID cannot be empty")
}

if errs := validateBackendMode(cfg.BackendMode); len(errs) > 0 {
if errs := mapper.ValidateBackendMode(cfg.BackendMode); len(errs) > 0 {
return cfg, utilerrors.NewAggregate(errs)
}

Expand All @@ -131,20 +130,3 @@ func getLogFormatter() logrus.Formatter {

return &logrus.TextFormatter{FullTimestamp: true}
}

func validateBackendMode(modes []string) []error {
var errs []error

allowedModes := sets.NewString(mapper.BackendModeChoices...)
for _, mode := range modes {
if !allowedModes.Has(mode) {
errs = append(errs, fmt.Errorf("backend-mode %q is not a valid mode", mode))
}
}

if len(modes) != len(sets.NewString(modes...).List()) {
errs = append(errs, fmt.Errorf("backend-mode %q has duplicates", modes))
}

return errs
}
7 changes: 5 additions & 2 deletions cmd/aws-iam-authenticator/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ var serverCmd = &cobra.Command{
logrus.Fatalf("%s", err)
}

mappers := server.BuildMapperChain(cfg)
mappers, err := server.BuildMapperChain(cfg)
if err != nil {
logrus.Fatalf("failed to build mapper chain: %v", err)
}
for _, m := range mappers {
logrus.Infof("starting mapper %q", m.Name())
if err := m.Start(stopCh); err != nil {
Expand Down Expand Up @@ -97,7 +100,7 @@ func init() {
viper.BindPFlag("server.address", serverCmd.Flags().Lookup("address"))

serverCmd.Flags().StringSlice("backend-mode",
[]string{mapper.ModeCRD, mapper.ModeConfigMap, mapper.ModeFile},
[]string{mapper.ModeMountedFile},
fmt.Sprintf("Ordered list of backends to get mappings from. The first one that returns a matching mapping wins. Comma-delimited list of: %s", strings.Join(mapper.BackendModeChoices, ",")))
wongma7 marked this conversation as resolved.
Show resolved Hide resolved
viper.BindPFlag("server.backendMode", serverCmd.Flags().Lookup("backend-mode"))

Expand Down
2 changes: 1 addition & 1 deletion pkg/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ type Config struct {
// +optional
Kubeconfig string

// BackendMode is an ordered list of backends to get mappings from. Comma-delimited list of: File,ConfigMap,CRD
// BackendMode is an ordered list of backends to get mappings from. Comma-delimited list of: MountedFile,EKSConfigMap,CRD
BackendMode []string

// Ec2 DescribeInstances rate limiting variables initially set to defaults until we completely
Expand Down
2 changes: 1 addition & 1 deletion pkg/mapper/configmap/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func NewConfigMapMapper(cfg config.Config) (*ConfigMapMapper, error) {
}

func (m *ConfigMapMapper) Name() string {
return mapper.ModeConfigMap
return mapper.ModeEKSConfigMap
}

func (m *ConfigMapMapper) Start(stopCh <-chan struct{}) error {
Expand Down
9 changes: 5 additions & 4 deletions pkg/mapper/crd/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (

type CRDMapper struct {
*controller.Controller
// iamInformerFactory is an informer factory that must be Started
iamInformerFactory informers.SharedInformerFactory
// iamMappingsSynced is a function to get if the informers have synced
iamMappingsSynced cache.InformerSynced
// iamMappingsIndex is a custom indexer which allows for indexing on canonical arns
Expand Down Expand Up @@ -61,7 +63,7 @@ func NewCRDMapper(cfg config.Config) (*CRDMapper, error) {

ctrl := controller.New(kubeClient, iamClient, iamMappingInformer)

return &CRDMapper{ctrl, iamMappingsSynced, iamMappingsIndex}, nil
return &CRDMapper{ctrl, iamInformerFactory, iamMappingsSynced, iamMappingsIndex}, nil
wongma7 marked this conversation as resolved.
Show resolved Hide resolved
}

func NewCRDMapperWithIndexer(iamMappingsIndex cache.Indexer) *CRDMapper {
Expand All @@ -73,15 +75,14 @@ func (m *CRDMapper) Name() string {
}

func (m *CRDMapper) Start(stopCh <-chan struct{}) error {
m.iamInformerFactory.Start(stopCh)
go func() {
// Run starts worker goroutines and blocks
if err := m.Controller.Run(2, stopCh); err != nil {
panic(err)
}
}()
if ok := cache.WaitForCacheSync(stopCh, m.iamMappingsSynced); !ok {
return fmt.Errorf("failed to wait for caches to sync")
}

return nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/mapper/file/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func NewFileMapperWithMaps(
}

func (m *FileMapper) Name() string {
return mapper.ModeFile
return mapper.ModeMountedFile
}

func (m *FileMapper) Start(_ <-chan struct{}) error {
Expand Down
50 changes: 47 additions & 3 deletions pkg/mapper/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,34 @@ package mapper

import (
"errors"
"fmt"

"github.com/sirupsen/logrus"
"k8s.io/apimachinery/pkg/util/sets"
"sigs.k8s.io/aws-iam-authenticator/pkg/config"
)

const (
ModeFile string = "File"
// Deprecated: use ModeMountedFile instead
ModeFile string = "File"
// Deprecated: use ModeEKSConfigMap instead
ModeConfigMap string = "ConfigMap"
ModeCRD string = "CRD"

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: just File might be more techincally correct since it doesn't have to run in a container, but I understand that it probably makes more sense in most cases, so MountedFile might be the better option.

ModeMountedFile string = "MountedFile"

ModeEKSConfigMap string = "EKSConfigMap"

ModeCRD string = "CRD"
)

var BackendModeChoices = []string{ModeFile, ModeConfigMap, ModeCRD}
var (
ValidBackendModeChoices = []string{ModeFile, ModeConfigMap, ModeMountedFile, ModeEKSConfigMap, ModeCRD}
DeprecatedBackendModeChoices = map[string]string{
ModeFile: ModeMountedFile,
ModeConfigMap: ModeEKSConfigMap,
}
BackendModeChoices = []string{ModeMountedFile, ModeEKSConfigMap, ModeCRD}
)

var ErrNotMapped = errors.New("ARN is not mapped")

Expand All @@ -23,3 +40,30 @@ type Mapper interface {
Map(canonicalARN string) (*config.IdentityMapping, error)
IsAccountAllowed(accountID string) bool
}

func ValidateBackendMode(modes []string) []error {
var errs []error

validModes := sets.NewString(ValidBackendModeChoices...)
for _, mode := range modes {
if !validModes.Has(mode) {
errs = append(errs, fmt.Errorf("backend-mode %q is not a valid mode", mode))
}
}

for _, mode := range modes {
if replacementMode, ok := DeprecatedBackendModeChoices[mode]; ok {
logrus.Warningf("warning: backend-mode %q is deprecated, use %q instead", mode, replacementMode)
}
}

if len(modes) != sets.NewString(modes...).Len() {
errs = append(errs, fmt.Errorf("backend-mode %q has duplicates", modes))
}

if len(modes) == 0 {
errs = append(errs, fmt.Errorf("at least one backend-mode must be specified"))
}

return errs
}
59 changes: 59 additions & 0 deletions pkg/mapper/mapper_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package mapper

import (
"testing"

"sigs.k8s.io/aws-iam-authenticator/pkg/config"
)

func TestValidateBackendMode(t *testing.T) {
cases := []struct {
name string
cfg config.Config
wantErrs bool
}{
{
name: "valid backend mode",
cfg: config.Config{
BackendMode: []string{ModeMountedFile, ModeEKSConfigMap, ModeCRD},
},
},
{
name: "valid deprecated backend mode",
cfg: config.Config{
BackendMode: []string{ModeFile, ModeConfigMap},
},
},
{
name: "invalid backend mode",
cfg: config.Config{
BackendMode: []string{"ModeFoo"},
},
wantErrs: true,
},
{
name: "empty backend mode",
cfg: config.Config{
BackendMode: []string{},
},
wantErrs: true,
},
{
name: "duplicate backend mode",
cfg: config.Config{
BackendMode: []string{ModeMountedFile, ModeMountedFile},
},
wantErrs: true,
},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
errs := ValidateBackendMode(c.cfg.BackendMode)
if len(errs) > 0 && !c.wantErrs {
t.Errorf("wanted no errors but got: %v", errs)
} else if len(errs) == 0 && c.wantErrs {
t.Errorf("wanted errors but got none")
}
})
}
}
16 changes: 10 additions & 6 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,34 +202,38 @@ func createMetrics() metrics {
return m
}

func BuildMapperChain(cfg config.Config) []mapper.Mapper {
func BuildMapperChain(cfg config.Config) ([]mapper.Mapper, error) {
modes := cfg.BackendMode
mappers := []mapper.Mapper{}
for _, mode := range modes {
switch mode {
case mapper.ModeFile:
fallthrough
case mapper.ModeMountedFile:
fileMapper, err := file.NewFileMapper(cfg)
if err != nil {
logrus.Fatalf("backend-mode %q creation failed: %v", mode, err)
return nil, fmt.Errorf("backend-mode %q creation failed: %v", mode, err)
wongma7 marked this conversation as resolved.
Show resolved Hide resolved
}
mappers = append(mappers, fileMapper)
case mapper.ModeConfigMap:
fallthrough
case mapper.ModeEKSConfigMap:
configMapMapper, err := configmap.NewConfigMapMapper(cfg)
if err != nil {
logrus.Fatalf("backend-mode %q creation failed: %v", mode, err)
return nil, fmt.Errorf("backend-mode %q creation failed: %v", mode, err)
}
mappers = append(mappers, configMapMapper)
case mapper.ModeCRD:
crdMapper, err := crd.NewCRDMapper(cfg)
if err != nil {
logrus.Fatalf("backend-mode %q creation failed: %v", mode, err)
return nil, fmt.Errorf("backend-mode %q creation failed: %v", mode, err)
}
mappers = append(mappers, crdMapper)
default:
logrus.Fatalf("backend-mode %q is not a valid mode", mode)
return nil, fmt.Errorf("backend-mode %q is not a valid mode", mode)
}
}
return mappers
return mappers, nil
}

func duration(start time.Time) float64 {
Expand Down