jfrog / terraform-provider-xray

Terraform provider to manage JFrog Xray
https://jfrog.com/xray/
Apache License 2.0
149 stars 12 forks source link

xray_ignore_rule error out when cves and vulnerabilities #148

Closed Osazz closed 10 months ago

Osazz commented 10 months ago

Describe the bug TF resource xray_ignore_rule fails on plan when cves = [] and vulnerabilities = ["XRAY-170461"] with Conflicting configuration arguments

Requirements for and issue

Terraform Code

resource "xray_ignore_rule" "ignore_rule" {
  notes           = "delete me now test ignore rule iac"
  vulnerabilities = ["XRAY-170461"]
  cves = [] 

}


**Additional context**
If you create ignore rule from UI and you import it using `terraform import xray_ignore_rule.ignore_rule <rule-id>`
- Your state file will have `"cves": null,`
- If you do a plan after import, you will get 1 destroyed and `cves` value changed to `[]`
- So this is is conflicting 
alexhung commented 10 months ago

@Osazz In your example, since you don't want to have any cves then you should omit it in the TF configuration. This will also match the imported value of null.

Osazz commented 10 months ago

@Osazz In your example, since you don't want to have any cves then you should omit it in the TF configuration. This will also match the imported value of null.

Here is my full use case , I have existing ignore rules that were created from the UI that I would like to manage using Terraform. Here are the steps I took and how I endup here :

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


N.B running apply means new destroying existing resources and creating new one. Not ideal

- Do terraform apply and then check the state file 

"instances": [ { "schema_version": 0, "attributes": { ... "cves": [], ... } } ]



So that is why I setting `cves as []` in the resources so that I dont have to destroy existing resource and I think it should be allowed
alexhung commented 10 months ago

@Osazz Just so I understand correctly, the TF config you have before you import the resource contains cves: []? i.e.

resource "xray_ignore_rule" "ignore_rule" {
  notes           = "delete me now test ignore rule iac"
  vulnerabilities = ["XRAY-170461"]
  cves = [] 
}

If you omit cves attribute in your config, like:

resource "xray_ignore_rule" "ignore_rule" {
  notes           = "delete me now test ignore rule iac"
  vulnerabilities = ["XRAY-170461"]
}

Then import the resource. After that terraform plan shows updates for cves attribute?

The Xray APIs don't allow updating an existing ignore rule. Thus any mismatch of TF configuration and API data will mean the provider destroys and recreates new resource.

Osazz commented 10 months ago

@alexhung No that understanding is not correct. I have a TF config like this :

resource "xray_ignore_rule" "ignore_rule" {
  notes           = "delete me now test ignore rule iac"
  vulnerabilities = ["XRAY-170461"]
}

Terraform will perform the following actions:

xray_ignore_rule.ignore_rule must be replaced

-/+ resource "xray_ignore_rule" "ignore_rule" { ~ author = "someauthor" -> (known after apply) ~ created = "2023-11-17T18:30:29Z" -> (known after apply)

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

Changes to Outputs: ~ author = "someauthor" -> (known after apply) ~ created = "2023-11-17T18:30:29Z" -> (known after apply) ~ id = "91ced9b-bb1e-4d9c-488c-6bf093a36d64" -> (known after apply) ~ is_expired = false -> (known after apply)


- When I go ahead to apply : I get this state file 

``` Terraform 
"instances": [
  {
    "schema_version": 0,
    "attributes": {
      ...
      "cves": [],
      ...
    }
  }
]

What could have stopped the difference would have been me been able to give cves value as [] but then I am running into that conflict error which does not seems to be a conflict as cves = [] is not same as vulnerabilities = ["XRAY-170461"]

The only work around which I could think of was to change the state file manual by making cves=[]. that resulted in No changes. Your infrastructure matches the configuration. but of course this is a bad practice and state file should never be manually updated.

alexhung commented 10 months ago

@Osazz I see. Thanks for the clarification! I'll investigate this issue.

ipowellBT commented 10 months ago

@alexhung any update on this issue?