Skip to content

Conversation

@jrobison-sb
Copy link
Contributor

Issue: N/A

Description:
limit_num_imgs should be user configurable. This adds it as a variable so that a user can configure it.

Additional Info:
N/A

variables.tf Outdated
default = 5
description = "The maximum number of newest container images to assess per repository. Must be one of 5, 10, or 15. Defaults to 5."

validation {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validation requires Terraform 0.13 or higher. I can remove this if desired.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jrobison-sb I think we should add any validation changes to the provider -> https://github.com/lacework/terraform-provider-lacework/blob/main/lacework/resource_lacework_integration_ecr.go#L179

Can you raise an issue against the Lacework Terraform provider instead of adding it in this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmurray-lacework Thanks for the feedback. I raised lacework/terraform-provider-lacework#270 and removed the validation from variables.tf.

@jrobison-sb
Copy link
Contributor Author

@dmurray-lacework Hi. Any thoughts on this?

@dmurray-lacework
Copy link
Collaborator

Hey @jrobison-sb LGTM just one comment on the validation.

@dmurray-lacework
Copy link
Collaborator

make it so

@dmurray-lacework dmurray-lacework merged commit 17571e1 into lacework:main Feb 21, 2022
@jrobison-sb
Copy link
Contributor Author

jrobison-sb commented Feb 21, 2022

@dmurray-lacework thanks for merging this. would it be possible to get a new release of the provider module so we could start using this?

@lacework-releng lacework-releng mentioned this pull request Feb 22, 2022
@dmurray-lacework
Copy link
Collaborator

@jrobison-sb We've just triggered the release -> #26

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 this pull request may close these issues.

2 participants