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

Bug - EKS can not create load balancers after module provisioned in new AWS account #87

Closed
1 task done
mmcaya opened this issue Aug 9, 2018 · 15 comments
Closed
1 task done

Comments

@mmcaya
Copy link
Contributor

mmcaya commented Aug 9, 2018

I have issues

Provisioning EKS cluster in new AWS account will result in an error when attempting to provision a load balancer if no load balancers of any kind have been provisioned before.

I'm submitting a...

  • bug report

What is the current behavior?

No previous load balancers ( i.e. service-link role AWSServiceRoleForElasticLoadBalancing doesn't exist)

AccessDenied: User: <MODULE-PROVISIONED-ROLE> is not authorized to perform: iam:CreateServiceLinkedRole on resource: arn:aws:iam::<ACCOUNT-ID>:role/aws-service-role/elasticloadbalancing.amazonaws.com/AWSServiceRoleForElasticLoadBalancing

because EKS is attempting to create the ELB service-link role for you, and the roles created by the module lack iam:CreateServiceLinkedRole

If this is a bug, how to reproduce? Please include a code sample if relevant.

  • Provision EKS cluster using module into new account (or ensure service-link role AWSServiceRoleForElasticLoadBalancing doesn't exist)
  • Attempt to provision a Service of type LoadBalancer via kubernetes
apiVersion: apps/v1
kind: Deployment
metadata:
  name: nginx-deployment
spec:
  selector:
    matchLabels:
      app: nginx
  replicas: 1 
  template:
    metadata:
      labels:
        app: nginx
    spec:
      containers:
      - name: nginx
        image: nginx:1.15.2
        ports:
        - containerPort: 80
---
kind: Service
apiVersion: v1
metadata:
  name: nginxservice
spec:
  type : LoadBalancer
  selector:
    app: nginx
  ports:
  - protocol: TCP
    port: 80
    targetPort: 80

What's the expected behavior?

EKS should provision load balancer.

Module should optionally provision (via flag) a resource "aws_iam_service_linked_role", or include updated IAM policies (iam:CreateServiceLinkedRole) to allow the EKS cluster to provision the required service-link role. Alternatively, if this is deemed not the responsibility of the module, the "Assumptions" section in README.md should note the issue.

Are you able to fix this problem and submit a PR? Link here if you have already.

Possibly, depending on the choice of solution (implementation change, documentation update)

Environment details

  • Affected module version: All

Any other relevant info

AWS Service Link FAQ:
https://docs.aws.amazon.com/elasticloadbalancing/latest/userguide/elb-service-linked-roles.html#create-service-linked-role

@max-rocket-internet
Copy link
Contributor

I had this too.

We need to add this either in this module or elsewhere in your TF codebase:

resource "aws_iam_service_linked_role" "elasticloadbalancing" {
  aws_service_name = "elasticloadbalancing.amazonaws.com"
}

@brandonjbjelland
Copy link
Contributor

Thanks for raising this @mmcaya

^ @max-rocket-internet nice. If only AWS had an explicit call for enabling a specific service that looked a little less indirect than this (GCP has these).

I wonder how the above behaves against an account that already has the service enabled... nothing I would think. There's likely potential for name conflict but probably I would guess no other side affects/risks.

Seeing that multiple developers have run into this AND now reported it, would guess that's just the tip of the iceberg. I'd accept a PR for it.

@max-rocket-internet
Copy link
Contributor

@mmcaya can you test this?

#91

@mmcaya
Copy link
Contributor Author

mmcaya commented Aug 16, 2018

@max-rocket-internet thanks, going to take a look at it later today.

@mmcaya
Copy link
Contributor Author

mmcaya commented Aug 17, 2018

Quick test confirms aws_iam_service_linked_role for elasticloadbalancing.amazonaws.com is not idempotent and does not support custom_suffix

To not be a BC break with the module, it seems that the default value of a flag would need to be false to prevent errors for those accounts already containing the role.

@max-rocket-internet @brandoconnor thoughts?

@max-rocket-internet
Copy link
Contributor

OK no worries, I've set the default to false.

@max-rocket-internet
Copy link
Contributor

@mmcaya can we close this after a fix was included in the latest release?

@max-rocket-internet
Copy link
Contributor

Should be resolved now in version v1.5.

@mmcaya
Copy link
Contributor Author

mmcaya commented Sep 4, 2018

👍 thanks for the fix

@johnharris85
Copy link

@max-rocket-internet was this reverted? What was the reason behind that? Inability to create resources like ELB etc... that are part of the aws-cloud-provider seems like a gap?

@mmcaya
Copy link
Contributor Author

mmcaya commented Feb 6, 2019

@johnharris85 see this comment from a separate issue: #132 (comment)

With AWS updating the default policy to include the ability to provision this resource if not present, it became a moot feature, and the module now defers back to EKS (or something else outside the module) to perform this action if needed.

@johnharris85
Copy link

johnharris85 commented Feb 6, 2019

Hmm thanks @mmcaya, I had issues yesterday (using latest version of the module) whereby ELBs, NLBs couldn't be created as a bunch of EC2 permissions were missing (e.g. ec2:DescribeInstanceAttribute). I worked around it by manually adding the policy specified here (https:/kubernetes/cloud-provider-aws#iam-policy + some tweaks) to the EKS control-plane IAM role. I originally came to this issue as I was about to submit a PR to pull through the EKS IAM role name as that's required to automate my workaround.

@mmcaya
Copy link
Contributor Author

mmcaya commented Feb 6, 2019

@johnharris85 there was also this issue and thread related specifically to ec2:DescribeInstanceAttribute and a few other missing permissions from the AWS documentation.
#183 (comment)

At this time, AWS has still not added those to the default cluster policy, but they were alerted at the time of the original issue being reported.

@johnharris85
Copy link

Awesome thanks! As a workaround-enabler (but also useful for other things I'm guessing, e.g. just wanting to add arbitrary policies to the control plane), would you be open to a PR to pull through the cluster IAM role name as an output? (implemented here -> master...johnharris85:pull-through-cluster-role-name).

Would allow folks to get around this by adding those extra policies in the near term, but also just generally useful going forward?

@github-actions
Copy link

github-actions bot commented Dec 2, 2022

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants