IAMIdentityMapping CRD Implementation #116

christopherhein · 2018-07-07T00:33:10Z

This adds an implementation of the CRD + Informer configuration for the authenticator. It implements the following object and will load the config.yaml file first into memory allowing existing clients to update then gracefully move over to the new mechanism.

---
apiVersion: authenticator.aws/v1alpha1
kind: IAMIdentityMapping
metadata:
  name: kubernetes-admin
spec:
  arn: arn:aws:iam::00000000000:role/KubernetesAdmin
  username: kubernetes-admin
  groups:
  - system:masters

It uses the Kubernetes codegen libraries to generate all the informers for updating the spec you just have to run ./codegen.sh

Also merging the user and role maps into a single map based on the IAMIdentities

This also adds a way to run the server locally using a local kubeconfig and generate certificates bound to a specific IP address and hostname.

closes #79

k8s-ci-robot · 2018-07-08T12:47:04Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: helpdesk@rt.linuxfoundation.org

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

christopherhein · 2018-07-10T22:16:27Z

Updated with resolved tests.

cmd/aws-iam-authenticator/root.go

example.yaml

pkg/apis/authenticator.aws/register.go

pkg/apis/authenticator.aws/v1alpha1/types.go

pkg/server/server.go

pkg/config/role_mappings.go

pkg/apis/authenticator.aws/v1alpha1/types.go

pkg/controller/controller.go

mattlandis

Good start. I'd still like to take a look at us moving to a single mapping for the CRDs (remove references to Role) and we need to to proper locking around the data structure.

If you have any questions on either these let me know.

pkg/apis/authenticator.aws/register.go

pkg/apis/authenticator.aws/v1alpha1/zz_generated.deepcopy.go

pkg/apis/authenticator.aws/register.go

christopherhein · 2018-08-07T11:27:14Z

@mattlandis Updated the commit history so it follows the format above, sorry for the delay on that. Still need to work on the locking for the config object.

About the removing role references, besides where we fall back to the static file for the time being the role vs user arn should have all been merged, did I miss something?

The only kind that doesn't naturally fit is accountIDs if you either have thoughts about that I kind of feel like a secondary resource might be useful there unless especially with trying to utilize the standard CRD validations (which still need to be implemented) - f0b3d2e#diff-786be668b558e47d16aef0627155a45bR78

nckturner · 2018-08-10T12:33:00Z

/assign

nckturner

Some Initial Feedback

example.yaml

nckturner · 2018-08-10T16:37:54Z

pkg/config/types.go

@@ -109,4 +109,7 @@ type Config struct {
 	// Bind defines the hostname or IP Address to bind the HTTPS server to listen to. This is useful when creating
 	// a local server to handle the authentication request for development.
 	Bind string
+
+	// CanonicalRoleMappings is a shared RoleMapping with keys based on the rolearns


It also removes 'Role' from the AWS IAM side of the naming which may help reduce confusion with the RBAC roles. It is either ARN or IAMARN or IAMidentity, all of which exist only on the AWS side.

Did we make any progress on this one? I'd agree with the benefit of getting rid of references to Role where possible. I'm guessing Role here refers to users, too?

pkg/controller/controller.go

christopherhein · 2019-06-01T13:16:46Z

It needs docs still but the feature is tested and working with the new informer set up if you’re interesting in giving it a CR I’d welcome that @errordeveloper

errordeveloper · 2019-06-01T13:24:03Z

Is it testable on EKS?

…

On Sat, 1 Jun 2019, 2:16 pm Christopher Hein, ***@***.***> wrote: It needs docs still but the feature is tested and working with the new informer set up if you’re interesting in giving it a CR I’d welcome that @errordeveloper <https://github.com/errordeveloper> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#116?email_source=notifications&email_token=AAB5MSYILSRPJYW2NSJLCMDPYJZEBA5CNFSM4FIYU3F2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWXAPHA#issuecomment-497944476>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAB5MSZV35IXVJZIJ5FXTD3PYJZEBANCNFSM4FIYU3FQ> .

christopherhein · 2019-06-01T13:44:35Z

No, since you can’t reconfigure the webhook. I typically just setup a kops cluster with thw authenticator, then I redeploy my config over it

christopherhein · 2019-06-01T14:03:26Z

@mumoshu any chance you could test this with kube-aws or on a kops cluster, if you have time...

nckturner

A few questions: 1. Are there README changes as a part of this PR yet? 2. It looks like you've removed the configuration fields from the file as a part of this PR, do you have an upgrade story yet? I was thinking we would need to deprecate them first.

nckturner · 2019-06-05T18:29:47Z

cmd/aws-iam-authenticator/root.go

@@ -88,12 +88,6 @@ func getConfig() (config.Config, error) {
 		Kubeconfig:                        viper.GetString("server.kubeconfig"),
 		Master:                            viper.GetString("server.master"),
 	}
-	if err := viper.UnmarshalKey("server.mapRoles", &config.RoleMappings); err != nil {


Do you have thoughts on upgrade? My first thought is that we should keep these in a deprecated state for the first version. If the mappings in the config are detected, only the config file is used and it takes precedence. This prevents the authenticator from failing on startup if the kubernetes config is not present but mappings in the config file are. Otherwise, upgrade is a bit more difficult.

yeah, that's a good point.

nckturner · 2019-06-05T19:39:29Z

example-role.yaml

@@ -0,0 +1,10 @@
+---


Maybe put the examples in an examples dir?

Sounds good!

nckturner · 2019-06-05T19:42:25Z

cmd/aws-iam-authenticator/server.go

-		"b",
+	serverCmd.Flags().String("kubeconfig",
+		"",
+		"kubeconfig file path for using a local kubeconfig to configure the client.")


This flag is a bit confusing due to the other kubeconfig flags which don't refer to the same kubeconfig. Can you specify in the description that "configure the client" means that we are talking about this server (the authenticator server) talking to an API server for mapping configuration?

nckturner · 2019-06-05T19:47:21Z

example.yaml

@@ -18,6 +18,80 @@
 # may also need to rework other bits to work in your cluster (e.g., node labels).
 #
 # This was tested with a kubeadm-installed cluster.
+---


I think you should separate out the identity mapping stuff from the daemonset in the example, as it may be common to run the authenticator in other ways. Additionally, for simplicity I see them as two separate steps in the setup guide, first applying everything the authenticator needs to run in your cluster, and second, running the authenticator itself.

Also we will need to change the daemonset container version at the bottom of this file.

nckturner · 2019-06-05T19:54:06Z

pkg/controller/controller.go

+	}
+
+	logrus.Info("setting up event handlers")
+	// adding the handler functions for the different IAM Identities, we can ignore deletes because we're only using the Listers from other objects


Can you explain a little more why your ignoring deletes?

Yeah, sounds good

cmd/aws-iam-authenticator/server.go

pkg/controller/controller.go

nckturner

A couple more comments.

pkg/apis/iamauthenticator/v1alpha1/types.go

pkg/controller/controller.go

example.yaml

pkg/server/server.go

nckturner · 2019-06-12T21:56:39Z

do you have an upgrade story yet?

We have been talking about our options in relation to EKS vs clusters using the open source authenticator. For the initial release of this feature, it will be protected behind a feature flag, and enabling it will be a binary switch for the authenticator server to look for mappings from CRDs. This will allow open source users to start using it and give us feedback. During this time, we will determine what the upgrade scenario for EKS will be from the EKS config map to custom resources.

Signed-off-by: Christopher Hein <me@chrishein.com>

christopherhein · 2019-06-13T12:11:07Z

@nckturner all updated with the latest fixes. Uses a feature-gate now, you'll notice I had to use the fake clients for when the flag was turned off, I think this will make long term clean up much easier.

to test, on a cluster for the authenticator config include:

--feature-gates=IAMIdentityMappingCRD

nckturner

Small nits but I think I'm going to merge and they can be addressed in a follow on PR.

nckturner · 2019-06-16T01:31:13Z

pkg/server/server.go

@@ -298,32 +328,69 @@ func (h *handler) authenticateEndpoint(w http.ResponseWriter, req *http.Request)
 }

 func (h *handler) doMapping(identity *token.Identity, arn string) (string, []string, error) {
-	if roleMapping, exists := h.lowercaseRoleMap[arn]; exists {
-		username, err := h.renderTemplate(roleMapping.Username, identity)
+	// IAMIdentityMappingCRD feature gate will only us the CRD implementation to validate users


nckturner · 2019-06-16T01:39:50Z

pkg/server/server.go

@@ -114,6 +124,7 @@ func (c *Server) Run() {
 			"groups":   mapping.Groups,
 		}).Infof("mapping IAM user")
 	}
+


Can we only iterate the users and roles from the config file if the feature flag is not enabled? Otherwise you get logs when the authenticator starts up that suggest it will be using the mappings from the config file in addition to the custom resources:

INFO[2019-06-15T06:12:21Z] mapping IAM role groups="[system:bootstrappers system:nodes system:node-proxier]" role="arn:aws:iam::..." username="system:node:{{EC2PrivateDNSName}}" INFO[2019-06-15T06:12:21Z] mapping IAM role groups="[system:masters]" role="arn:aws:iam::..." username="cross-account-{{SessionName}}" INFO[2019-06-15T06:12:21Z] mapping IAM role groups="[system:masters]" role="arn:aws:iam::..." username="admin:{{SessionName}}" INFO[2019-06-15T06:12:21Z] mapping IAM role groups="[system:masters]" role="arn:aws:iam::..." username=...

nckturner · 2019-06-16T06:43:16Z

/lgtm

metral · 2019-06-17T00:04:07Z

This change seems to have made the library error out on go get due to type mismatches. Any ideas?

christopherhein · 2019-06-17T00:15:48Z

@metral can you send your errors? Are you using go get sigs.k8s.io/aws-iam-authenticator

errordeveloper · 2019-06-17T08:54:13Z

🚀

metral · 2019-06-17T15:09:36Z

@christopherhein: running go get -u -v github.com/kubernetes-sigs/aws-iam-authenticator/cmd/aws-iam-authenticator to get the cmd returns:

...
go/src/github.com/kubernetes-sigs/aws-iam-authenticator/cmd/aws-iam-authenticator/root.go:68:18: cannot use config.DefaultFeatureGates (type map["sigs.k8s.io/aws-iam-authenticator/vendor/k8s.io/component-base/featuregate".Feature]"sigs.k8s.io/aws-iam-authenticator/vendor/k8s.io/component-base/featuregate".FeatureSpec) as type map["github.com/kubernetes-sigs/aws-iam-authenticator/vendor/k8s.io/component-base/featuregate".Feature]"github.com/kubernetes-sigs/aws-iam-authenticator/vendor/k8s.io/component-base/featuregate".FeatureSpec in argument to featureGates.Add
go/src/github.com/kubernetes-sigs/aws-iam-authenticator/cmd/aws-iam-authenticator/root.go:96:3: cannot use featureGates (type *"github.com/kubernetes-sigs/aws-iam-authenticator/vendor/k8s.io/component-base/featuregate".featureGate) as type "sigs.k8s.io/aws-iam-authenticator/vendor/k8s.io/component-base/featuregate".MutableFeatureGate in field value:
        *"github.com/kubernetes-sigs/aws-iam-authenticator/vendor/k8s.io/component-base/featuregate".featureGate does not implement "sigs.k8s.io/aws-iam-authenticator/vendor/k8s.io/component-base/featuregate".MutableFeatureGate (wrong type for Add method)
                have Add(map["github.com/kubernetes-sigs/aws-iam-authenticator/vendor/k8s.io/component-base/featuregate".Feature]"github.com/kubernetes-sigs/aws-iam-authenticator/vendor/k8s.io/component-base/featuregate".FeatureSpec) error
                want Add(map["sigs.k8s.io/aws-iam-authenticator/vendor/k8s.io/component-base/featuregate".Feature]"sigs.k8s.io/aws-iam-authenticator/vendor/k8s.io/component-base/featuregate".FeatureSpec) error
go/src/github.com/kubernetes-sigs/aws-iam-authenticator/cmd/aws-iam-authenticator/server.go:82:43: cannot use k8sconfig (type *"github.com/kubernetes-sigs/aws-iam-authenticator/vendor/k8s.io/client-go/rest".Config) as type *"sigs.k8s.io/aws-iam-authenticator/vendor/k8s.io/client-go/rest".Config in argument to versioned.NewForConfig
go/src/github.com/kubernetes-sigs/aws-iam-authenticator/cmd/aws-iam-authenticator/server.go:89:26: cannot use kubeClient (type "github.com/kubernetes-sigs/aws-iam-authenticator/vendor/k8s.io/client-go/kubernetes".Interface) as type "sigs.k8s.io/aws-iam-authenticator/vendor/k8s.io/client-go/kubernetes".Interface in argument to controller.New:
        "github.com/kubernetes-sigs/aws-iam-authenticator/vendor/k8s.io/client-go/kubernetes".Interface does not implement "sigs.k8s.io/aws-iam-authenticator/vendor/k8s.io/client-go/kubernetes".Interface (wrong type for AdmissionregistrationV1beta1 method)
                have AdmissionregistrationV1beta1() "github.com/kubernetes-sigs/aws-iam-authenticator/vendor/k8s.io/client-go/kubernetes/typed/admissionregistration/v1beta1".AdmissionregistrationV1beta1Interface
                want AdmissionregistrationV1beta1() "sigs.k8s.io/aws-iam-authenticator/vendor/k8s.io/client-go/kubernetes/typed/admissionregistration/v1beta1".AdmissionregistrationV1beta1Interface
go/src/github.com/kubernetes-sigs/aws-iam-authenticator/cmd/aws-iam-authenticator/server.go:102:56: cannot use []"github.com/kubernetes-sigs/aws-iam-authenticator/vendor/k8s.io/apimachinery/pkg/runtime".Object literal (type []"github.com/kubernetes-sigs/aws-iam-authenticator/vendor/k8s.io/apimachinery/pkg/runtime".Object) as type []"sigs.k8s.io/aws-iam-authenticator/vendor/k8s.io/apimachinery/pkg/runtime".Object in argument to "sigs.k8s.io/aws-iam-authenticator/pkg/generated/clientset/versioned/fake".NewSimpleClientset

dnascimento · 2019-08-21T14:49:07Z

When will the release v0.5 be available to deliver this PR as alpha? We are using a build of the master branch to test it

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 7, 2018

k8s-ci-robot requested review from mattlandis and nckturner July 7, 2018 00:33

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 7, 2018

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jul 8, 2018

christopherhein force-pushed the feature/crd-identity-mapping branch 2 times, most recently from 8cf7da5 to 7002524 Compare July 8, 2018 12:53

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jul 8, 2018

mattlandis reviewed Jul 11, 2018

View reviewed changes

christopherhein force-pushed the feature/crd-identity-mapping branch 3 times, most recently from a9aa336 to aaf8e42 Compare July 16, 2018 23:45

christopherhein changed the title ~~First Pass CRD Implementation [Looking for Feedback]~~ IAMIdentity CRD Implementation Jul 16, 2018

christopherhein mentioned this pull request Jul 17, 2018

userarn should be the key instead of userARN #114

Closed

christopherhein force-pushed the feature/crd-identity-mapping branch from aaf8e42 to 25b3bbc Compare July 23, 2018 21:59

christopherhein changed the title ~~IAMIdentity CRD Implementation~~ IAMIdentityMapping CRD Implementation Jul 23, 2018

christopherhein force-pushed the feature/crd-identity-mapping branch from 25b3bbc to 83b0ce4 Compare July 24, 2018 01:58

mattlandis reviewed Jul 29, 2018

View reviewed changes

pkg/apis/authenticator.aws/register.go Outdated Show resolved Hide resolved

pkg/apis/authenticator.aws/v1alpha1/zz_generated.deepcopy.go Outdated Show resolved Hide resolved

nckturner reviewed Aug 1, 2018

View reviewed changes

pkg/apis/authenticator.aws/register.go Outdated Show resolved Hide resolved

pkg/apis/authenticator.aws/register.go Outdated Show resolved Hide resolved

christopherhein force-pushed the feature/crd-identity-mapping branch 2 times, most recently from 9db0efe to 7cd562b Compare August 7, 2018 11:17

k8s-ci-robot assigned nckturner Aug 10, 2018

nckturner reviewed Aug 10, 2018

View reviewed changes

christopherhein force-pushed the feature/crd-identity-mapping branch from 7cd562b to cc9e507 Compare August 12, 2018 06:56

nckturner reviewed Jun 5, 2019

View reviewed changes

wongma7 reviewed Jun 10, 2019

View reviewed changes

cmd/aws-iam-authenticator/server.go Outdated Show resolved Hide resolved

pkg/controller/controller.go Outdated Show resolved Hide resolved

nckturner reviewed Jun 11, 2019

View reviewed changes

pkg/apis/iamauthenticator/v1alpha1/types.go Show resolved Hide resolved

pkg/controller/controller.go Show resolved Hide resolved

example.yaml Outdated Show resolved Hide resolved

pkg/server/server.go Outdated Show resolved Hide resolved

christopherhein mentioned this pull request Jun 11, 2019

Add Principle ID to CRD Implementation #234

Open

christopherhein force-pushed the feature/crd-identity-mapping branch from 3606cc1 to db96765 Compare June 13, 2019 11:27

adding controller and server code

a292286

Signed-off-by: Christopher Hein <me@chrishein.com>

christopherhein force-pushed the feature/crd-identity-mapping branch from db96765 to 648e742 Compare June 13, 2019 11:28

christopherhein added 2 commits June 13, 2019 04:32

adding vendored deps

d2d15c3

Signed-off-by: Christopher Hein <me@chrishein.com>

adding generated kubernetes types

2f5ffca

Signed-off-by: Christopher Hein <me@chrishein.com>

christopherhein force-pushed the feature/crd-identity-mapping branch from 648e742 to 97db681 Compare June 13, 2019 11:42

christopherhein added 2 commits June 13, 2019 05:07

adding documentation

be527c1

Signed-off-by: Christopher Hein <me@chrishein.com>

update goreleaser and travis

4217780

Signed-off-by: Christopher Hein <me@chrishein.com>

christopherhein force-pushed the feature/crd-identity-mapping branch from 4aadb65 to 4217780 Compare June 13, 2019 12:08

nckturner approved these changes Jun 16, 2019

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 16, 2019

k8s-ci-robot merged commit 7eb3cea into kubernetes-sigs:master Jun 16, 2019

christopherhein mentioned this pull request Jul 26, 2019

How to update user's permissions immediately without manually delete pods #175

Closed

rndstr mentioned this pull request Sep 12, 2019

Feature/iam users eksctl-io/eksctl#932

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IAMIdentityMapping CRD Implementation #116

IAMIdentityMapping CRD Implementation #116

christopherhein commented Jul 7, 2018 •

edited

Loading

k8s-ci-robot commented Jul 8, 2018

christopherhein commented Jul 10, 2018

mattlandis left a comment

christopherhein commented Aug 7, 2018 •

edited

Loading

nckturner commented Aug 10, 2018

nckturner left a comment

nckturner Aug 10, 2018

christopherhein commented Jun 1, 2019

errordeveloper commented Jun 1, 2019 via email

christopherhein commented Jun 1, 2019 •

edited

Loading

christopherhein commented Jun 1, 2019

nckturner left a comment

nckturner Jun 5, 2019

christopherhein Jun 6, 2019

nckturner Jun 5, 2019

christopherhein Jun 6, 2019

nckturner Jun 5, 2019

christopherhein Jun 6, 2019

nckturner Jun 5, 2019

nckturner Jun 5, 2019

nckturner Jun 5, 2019

christopherhein Jun 6, 2019

nckturner left a comment

nckturner commented Jun 12, 2019

christopherhein commented Jun 13, 2019

nckturner left a comment

nckturner Jun 16, 2019

nckturner Jun 16, 2019

nckturner commented Jun 16, 2019

metral commented Jun 17, 2019

christopherhein commented Jun 17, 2019

errordeveloper commented Jun 17, 2019 •

edited

Loading

metral commented Jun 17, 2019 •

edited

Loading

dnascimento commented Aug 21, 2019

IAMIdentityMapping CRD Implementation #116

IAMIdentityMapping CRD Implementation #116

Conversation

christopherhein commented Jul 7, 2018 • edited Loading

k8s-ci-robot commented Jul 8, 2018

christopherhein commented Jul 10, 2018

mattlandis left a comment

Choose a reason for hiding this comment

christopherhein commented Aug 7, 2018 • edited Loading

nckturner commented Aug 10, 2018

nckturner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christopherhein commented Jun 1, 2019

errordeveloper commented Jun 1, 2019 via email

christopherhein commented Jun 1, 2019 • edited Loading

christopherhein commented Jun 1, 2019

nckturner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nckturner left a comment

Choose a reason for hiding this comment

nckturner commented Jun 12, 2019

christopherhein commented Jun 13, 2019

nckturner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nckturner commented Jun 16, 2019

metral commented Jun 17, 2019

christopherhein commented Jun 17, 2019

errordeveloper commented Jun 17, 2019 • edited Loading

metral commented Jun 17, 2019 • edited Loading

dnascimento commented Aug 21, 2019

christopherhein commented Jul 7, 2018 •

edited

Loading

christopherhein commented Aug 7, 2018 •

edited

Loading

christopherhein commented Jun 1, 2019 •

edited

Loading

errordeveloper commented Jun 17, 2019 •

edited

Loading

metral commented Jun 17, 2019 •

edited

Loading