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

Change HCL for adding resources to watches #4

Closed
jasonwbarnett opened this issue Apr 26, 2021 · 4 comments
Closed

Change HCL for adding resources to watches #4

jasonwbarnett opened this issue Apr 26, 2021 · 4 comments
Assignees

Comments

@jasonwbarnett
Copy link
Contributor

Currently the API for adding a repository (or any type of resource) is like so:

resource "artifactory_xray_watch" "test" {
  name        = "watch-npm-local-repo"
  description = "apply a severity-based policy to the npm local repo"

  resources {
    type       = "repository"
    name       = "npm-local"
    bin_mgr_id = "default"
    repo_type  = "local"
  }
}

I strongly believe that the resources key should be singular (i.e. resource) since it is intended to be specified once per resource you intend to add to the xray watch. This would make this more consistent with the core terraform modules. Given that resource is

resource "artifactory_xray_watch" "test" {
  name        = "watch-npm-local-repo"
  description = "apply a severity-based policy to the npm local repo"

  resource {
    type       = "repository"
    name       = "npm-local"
    bin_mgr_id = "default"
    repo_type  = "local"
  }

  resource {
    type       = "repository"
    name       = "npm-remote"
    bin_mgr_id = "default"
    repo_type  = "remote"
  }
}
@skyzyx
Copy link

skyzyx commented Oct 1, 2021

I imagine this is a breaking change, so it should be deferred to v3.0.0. But I think it's a good idea.

@chb0github
Copy link
Contributor

Yes, this would be breaking. I have actually spoken @jasonwbarnett. Right now I have an open discussion ticket about moving the xray code out into it's own provider. And, at that point, I think it would be a fair discussion to change the HCL.

I didn't come up with this HCL, and my editor gives me warnings about it. But I can't just break people. Please upvote and give your comments on the discussion to move xray to it's own provider

@chb0github
Copy link
Contributor

I'd like to get your vote on this
jfrog/terraform-provider-artifactory#160

@danielmkn danielmkn self-assigned this Oct 20, 2021
@danielmkn danielmkn transferred this issue from jfrog/terraform-provider-artifactory Nov 2, 2021
@danielmkn danielmkn changed the title API for adding resources to watches isn't intuitive Change HCL for adding resources to watches Nov 2, 2021
danielmkn added a commit that referenced this issue Dec 2, 2021
- Add more tests.
- Replace plurals with single form - resource, filter, assigned_policy per PTRENG-3122 and GH-4
- Remove old tests and filter wrapper
danielmkn added a commit that referenced this issue Dec 14, 2021
- Xray API v2 support for watches and policies
- Use of singular key names instead of plural
- Use TypeSet instead of TypeList to prevent sorting problems in TF state with multiple elements
- HCL changes
- #1, #2, #4
@danielmkn
Copy link
Collaborator

Released in v0.0.1

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.

4 participants