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

resource/aws_ssm_document: Issues with Document Permissions #5308

Closed
brandonstevens opened this issue Jul 24, 2018 · 3 comments
Closed

resource/aws_ssm_document: Issues with Document Permissions #5308

brandonstevens opened this issue Jul 24, 2018 · 3 comments
Labels
bug Addresses a defect in current functionality. service/ssm Issues and PRs that pertain to the ssm service. stale Old or inactive issues managed by automation, if no further action taken these will get closed.

Comments

@brandonstevens
Copy link
Contributor

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform Version

$ terraform -v
Terraform v0.11.7
+ provider.aws v1.28.0

Affected Resource(s)

  • aws_ssm_document

Terraform Configuration Files

terraform {
  required_version = "~> 0.11.7"
}

provider "aws" {
  version = "~> 1.28.0"
}

resource "aws_ssm_document" "foo" {
  name          = "test_document"
  document_type = "Command"

  permissions {
    type = "Share"

    # random AWS accounts
    account_ids = "118535508916,154276429936"
  }

  content = <<DOC
  {
    "schemaVersion": "1.2",
    "description": "Check ip configuration of a Linux instance.",
    "parameters": {

    },
    "runtimeConfig": {
      "aws:runShellScript": {
        "properties": [
          {
            "id": "0.aws:runShellScript",
            "runCommand": ["ifconfig"]
          }
        ]
      }
    }
  }
DOC
}

Output

$ terraform plan
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

aws_ssm_document.foo: Refreshing state... (ID: test_document)

------------------------------------------------------------------------

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  ~ aws_ssm_document.foo
      permissions.account_ids: "118535508916,154276429936" => "118535508916"


Plan: 0 to add, 1 to change, 0 to destroy.

------------------------------------------------------------------------

Note: You didn't specify an "-out" parameter to save this plan, so Terraform
can't guarantee that exactly these actions will be performed if
"terraform apply" is subsequently run.

Expected Behavior

  • Terraform removes sharing permissions from AWS Accounts

Actual Behavior

  • Terraform fails to remove any AWS Accounts for sharing permissions

Steps to Reproduce

  1. terraform apply to create the initial state
  2. Remove one of the two AWS Accounts from the sharing permissions
  3. terraform apply to apply the above changes
  4. terraform plan to show above changes were not applied

Important Factoids

Reviewing the code for resourceAwsSsmDocumentUpdate and setDocumentPermissions, it looks like on resource updates, Terraform only attempts to add AWS Accounts. The AWS API for modifying Document permissions expects a separate list of Accounts to remove.

Also, there appears to be a related issue with removing all sharing permissions. Using the same code above, after removing the permissions block, Terraform detects the changes, but when it's run to apply them, it skips over modifying permissions.

2018/07/24 07:07:45 [DEBUG] apply: aws_ssm_document.foo: executing Apply
2018-07-24T07:07:45.724-0500 [DEBUG] plugin.terraform-provider-aws_v1.28.0_x4: 2018/07/24 07:07:45 [DEBUG] Not setting document permissions on "test_document"

Lastly, it's not possible to delete Documents without explicitly removing sharing permissions. I understand that this is how the AWS API works, but I think updating the provider to automatically remove all permissions on a delete would improve the user experience.

For these latter two issues, I'm happy to open separate issues if that's preferred.

@bflad bflad added bug Addresses a defect in current functionality. service/ssm Issues and PRs that pertain to the ssm service. labels Jul 24, 2018
@bflad
Copy link
Contributor

bflad commented Jul 25, 2018

For this item specifically:

Lastly, it's not possible to delete Documents without explicitly removing sharing permissions. I understand that this is how the AWS API works, but I think updating the provider to automatically remove all permissions on a delete would improve the user experience.

I'm not the most familiar with this specific resource, but from the sounds of it, I would agree that the correct approach would be to fix this within the resource (removing all permissions first) since the resource is what manages those permissions. Its interesting that TestAccAWSSSMDocument_permission does not catch this issue in our acceptance testing, but maybe its specifically around privately shared documents, which is not currently captured by an acceptance test.

@github-actions
Copy link

Marking this issue as stale due to inactivity. This helps our maintainers find and focus on the active issues. If this issue receives no comments in the next 30 days it will automatically be closed. Maintainers can also remove the stale label.

If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thank you!

@github-actions github-actions bot added the stale Old or inactive issues managed by automation, if no further action taken these will get closed. label Jul 14, 2020
@ghost
Copy link

ghost commented Sep 14, 2020

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Sep 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/ssm Issues and PRs that pertain to the ssm service. stale Old or inactive issues managed by automation, if no further action taken these will get closed.
Projects
None yet
Development

No branches or pull requests

2 participants