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

Removing aws_iam_service_linked_role from module #160

Merged
merged 2 commits into from
Oct 17, 2018
Merged

Removing aws_iam_service_linked_role from module #160

merged 2 commits into from
Oct 17, 2018

Conversation

max-rocket-internet
Copy link
Contributor

@max-rocket-internet max-rocket-internet commented Oct 11, 2018

PR o'clock

Description

Removing this resource. I think it's not required and seems to be the source of various errors anyway.

It was added here: #91

And issues here:
#132
#144

If anyone gets an error about removing the aws_iam_service_linked_role.elasticloadbalancing resource, I think you can just remove it from the state: terraform state rm aws_iam_service_linked_role.elasticloadbalancing

Checklist

  • terraform fmt and terraform validate both work from the root and examples/eks_test_fixture directories (look in CI for an example)
  • Tests for the changes have been added and passing (for bug fixes/features)
  • Test results are pasted in this PR (in lieu of CI)
  • Docs have been updated using terraform-docs per README.md instructions
  • I've added my change to CHANGELOG.md
  • Any breaking changes are highlighted above

@max-rocket-internet max-rocket-internet merged commit b15e1d7 into terraform-aws-modules:master Oct 17, 2018
@tonyxiao
Copy link

@max-rocket-internet this used to be required to fix the issue of EKS not being able to create ELB. Is that no longer an issue? How do you know we didn’t introduce a regression here?

nicgrayson pushed a commit to nicgrayson/terraform-aws-eks that referenced this pull request Oct 29, 2018
* upstream/master: (25 commits)
  Update documentation for removed `configure_kubectl_session` (terraform-aws-modules#171)
  remove the checksum step
  Add target_group_arns to worker ASG (terraform-aws-modules#167)
  Removing 2 providers from the module (terraform-aws-modules#168)
  Removing aws_iam_service_linked_role from module (terraform-aws-modules#160)
  Adjust the order and correct/update the info (terraform-aws-modules#163)
  Ruby ver `2.4.2` -> `2.4.4`
  Move env vars into env section
  Remove `v` in `v0.11.8`
  Better version control
  Add suspended_processes attributes to autoscaling_group (terraform-aws-modules#159)
  Updating changelog for v1.7.0 (terraform-aws-modules#158)
  Revert "Add suspended_processes attributes to autoscaling_group (terraform-aws-modules#153)" (terraform-aws-modules#157)
  Add suspended_processes attributes to autoscaling_group (terraform-aws-modules#153)
  Add option to change worker placement_tenancy. (terraform-aws-modules#142)
  Allowing 443 to nodes from EKS service (terraform-aws-modules#148)
  Fixed issue with 'workers_group_defaults_defaults.iam_role_id' and added explicit depends_on for 'update_config_map_aws_auth' (terraform-aws-modules#147)
  Added timeout configs and variables to aws_eks_cluster resource (terraform-aws-modules#149)
  Fixing travis config (terraform-aws-modules#151)
  Fix for ERROR: 'aws_iam_instance_profile.workers' not found (terraform-aws-modules#141)
  ...
@max-rocket-internet
Copy link
Contributor Author

Is that no longer an issue? How do you know we didn’t introduce a regression here?

Because I tested it. Also, the default AWS IAM policy arn:aws:iam::aws:policy/AmazonEKSClusterPolicy includes this now:

        {
            "Effect": "Allow",
            "Action": "iam:CreateServiceLinkedRole",
            "Resource": "*",
            "Condition": {
                "StringLike": {
                    "iam:AWSServiceName": "elasticloadbalancing.amazonaws.com"
                }
            }
        }

I would be curious if you still have that error when your cluster is using the default AWS IAM policy.

@tonyxiao
Copy link

Ah ok. Got it thank you.

@github-actions
Copy link

I'm going to lock this pull request 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 related to this change, 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 Nov 24, 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

Successfully merging this pull request may close these issues.

2 participants