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

Add the ability to mock return values of resources such as kms keys. #4

Closed
tomelliot16 opened this issue Sep 7, 2020 · 9 comments · Fixed by #12
Closed

Add the ability to mock return values of resources such as kms keys. #4

tomelliot16 opened this issue Sep 7, 2020 · 9 comments · Fixed by #12

Comments

@tomelliot16
Copy link

After playing around a bit somethings I have found that are missing. such as values returned from the provider once resource is created should probably be able to be mocked. Ex:

resource "aws_s3_bucket" "b" {
  bucket = "my-tf-test-bucket"
}

output "private_ip" {
  value = aws_s3_bucket.b.id
}

spec could be something like

assert "aws_s3_bucket" "c" {
  bucket = "my-tf-test-bucket"

  return {
    id = "arn:aws:s3::0000000000:my-tf-test-bucket"
  }
}

assert "output" "private_ip" {
    value = "arn:aws:s3::0000000000:my-tf-test-bucket"
}
@nhurel
Copy link
Owner

nhurel commented Sep 7, 2020

I understand the need of being able to test with specific resource attributes values but I don't think those values should be in the assert block. The assert block should only describe the expected result of applying the terraform config under certain circumstances (the input values and the mocked data source).

Do you have other example in mind where you'd like more control on the resource attributes ? That would help to design this feature

@tomelliot16
Copy link
Author

A real case I'm dealing with which came up with the thought is

resource "aws_kms_key" "key" {
  description = "KMS key 1"
}

resource "aws_iam_role_policy" "policy" {
  role = aws_iam_role.role.id
  policy = <<-EOF
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Action": [
                "kms:Decrypt",
                "kms:DescribeKey",
                "kms:Encrypt",
                "kms:GenerateDataKey*",
                "kms:ReEncrypt*"
            ],
            "Resource": [
                "${aws_kms_key.key.id}"
            ]
        }
  EOF
}

resource "aws_iam_role" "role" {
  name = "role"

  assume_role_policy = <<-EOF
    {
      "Version": "2012-10-17",
      "Statement": [
        {
          "Effect": "Allow",
          "Principal": {
            "AWS": "arn:aws:iam::${data.aws_caller_identity.current.account_id}:root"
          },
          "Action": "sts:AssumeRole",
          "Condition": {
            "Bool": {
              "aws:MultiFactorAuthPresent": "true"
            }
          }
        }
      ]
    }
    EOF
}

Where I want to assert each one of the above however when ids are not set they are only set as nil

aws_iam_role_policy.policy.role : <nil> != at-least-something-unique-for-role-arn

I also was thinking of asserting policies.

@tomelliot16
Copy link
Author

tomelliot16 commented Sep 7, 2020

Also I tend to disagree that is doesn't belong in the assert block because it seems the natural place when comparing it with rspec or other mocking systems
expect(thing).to receive(:call_name).with(args).and_return(something)
basically we are expecting the provider to fill in some data that will be used for other things. I'm not sure I understand why you don't want to add the return to the assert block.

@tomelliot16
Copy link
Author

tomelliot16 commented Sep 7, 2020

perhaps this would be fine

assert "aws_s3_bucket" "c" {
  bucket = "my-tf-test-bucket"
}
mock_resource "aws_s3_bucket" "c" {
  return {
    id = "arn:aws:s3::0000000000:my-tf-test-bucket"
  }
}

@tomelliot16
Copy link
Author

tomelliot16 commented Sep 8, 2020

if its language you could change it to

expect "aws_s3_bucket" "c" {
  bucket = "my-tf-test-bucket"
  return {
    id = "arn:aws:s3::0000000000:my-tf-test-bucket"
  }
}

@nhurel
Copy link
Owner

nhurel commented Sep 9, 2020

indeed, that's better with expect rather than assert. And it's easier to read (and write) with a single expect block rather than splitting it betwen an assert block and a mock one 👍

@nhurel
Copy link
Owner

nhurel commented Oct 25, 2020

Hello @tomelliot16
I've started implemented support for mocking return values or resources in the mock-return branch.
It's still work in progress but you can start testing it if you want

@tomelliot16
Copy link
Author

@nhurel I've gotten my team to work on this from Acquia acquia#1 I will open a PR to this repo

@tomelliot16
Copy link
Author

@nhurel nhurel linked a pull request Nov 2, 2020 that will close this issue
@nhurel nhurel closed this as completed in #12 Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants