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

CBR typeset change for cbr rule/zone and adding CBR retries #5246

Merged
merged 6 commits into from
Apr 23, 2024

Conversation

tyao117
Copy link
Contributor

@tyao117 tyao117 commented Mar 27, 2024

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #0000
Closes #4171
Closes #5002
Closes #4971

Output from acceptance testing:

$ make testacc TESTARGS='-run=TEST=./ibm/service/contextbasedrestrictions'
=== RUN   TestAccIBMCbrRuleDataSourceBasic
--- PASS: TestAccIBMCbrRuleDataSourceBasic (12.91s)
=== RUN   TestAccIBMCbrRuleDataSourceAllArgs
--- PASS: TestAccIBMCbrRuleDataSourceAllArgs (11.39s)
=== RUN   TestAccIBMCbrZoneDataSourceBasic
--- PASS: TestAccIBMCbrZoneDataSourceBasic (10.23s)
=== RUN   TestAccIBMCbrZoneDataSourceAllArgs
--- PASS: TestAccIBMCbrZoneDataSourceAllArgs (9.41s)
=== RUN   TestAccIBMCbrRuleBasic
--- PASS: TestAccIBMCbrRuleBasic (10.01s)
=== RUN   TestAccIBMCbrRuleAllArgs
--- PASS: TestAccIBMCbrRuleAllArgs (18.06s)
=== RUN   TestAccIBMCbrZoneBasic
--- PASS: TestAccIBMCbrZoneBasic (9.40s)
=== RUN   TestAccIBMCbrZoneAllArgs
--- PASS: TestAccIBMCbrZoneAllArgs (18.45s)
PASS

...

@tyao117 tyao117 changed the title CBR typeset change for cbr rule CBR typeset change for cbr rule/zone and adding CBR retries Mar 28, 2024
@tyao117
Copy link
Contributor Author

tyao117 commented Mar 28, 2024

showing the change of the retry:

...
{"trace":"3a1c7778-0c97-44e3-a46d-1ab29f7c6740","errors":[{"code":"rule_not_found_in_store","message":"The rule 'd38463f844a13ed98eb97c48f909cb5a' is not found in store.","target":{"type":"parameter","name":"rule_id","value":"d38463f844a13ed98eb97c48f909cb5a"}}],"status_code":404}: timestamp=2024-03-28T17:19:52.226-0500
2024-03-28T17:19:52.228-0500 [INFO]  provider.terraform-provider-ibm: Read cbr rule response status code: 404, provider will try again. The rule 'd38463f844a13ed98eb97c48f909cb5a' is not found in store.: timestamp=2024-03-28T17:19:52.228-0500
ibm_cbr_rule.cbr_rule: Still creating... [10s elapsed]
ibm_cbr_rule.cbr_rule: Still creating... [20s elapsed]
2024-03-28T17:20:12.231-0500 [INFO]  provider.terraform-provider-ibm: 2024/03/28 17:20:12 [Debug] Request:
GET /v1/rules/d38463f844a13ed98eb97c48f909cb5a HTTP/1.1
Host: testing-1-ap-south.network-policy.cloud.ibm.com
User-Agent: platform-services-go-sdk/0.61.2 (lang=go; arch=arm64; os=darwin; go.version=go1.21.6)
Accept: application/json
Authorization: [redacted]
X-Original-User-Agent: terraform-provider-ibm/1.64.0-beta0
Accept-Encoding: gzip
: timestamp=2024-03-28T17:20:12.231-0500
2024-03-28T17:20:12.231-0500 [INFO]  provider.terraform-provider-ibm: 2024/03/28 17:20:12 [DEBUG] GET https://testing-1-ap-south.network-policy.cloud.ibm.com/v1/rules/d38463f844a13ed98eb97c48f909cb5a: timestamp=2024-03-28T17:20:12.231-0500
2024-03-28T17:20:12.810-0500 [INFO]  provider.terraform-provider-ibm: 2024/03/28 17:20:12 [Debug] Response:
HTTP/2.0 200 OK
Content-Length: 973
Akamai-Grn: 0.b8764017.1711664412.1a11d10
Cache-Control: max-age=0, no-cache, no-store
Content-Type: application/json; charset=utf-8
Date: Thu, 28 Mar 2024 22:20:12 GMT
Etag: 1-e7fe6ccc1aa9b260cf9a75f5545918a6
Expires: Thu, 28 Mar 2024 22:20:12 GMT
Pragma: no-cache
Set-Cookie: ak_bmsc=37096D28113F036166B2AA3D9403EEE0~000000000000000000000000000000~YAAQuHZAFxv5bIWOAQAAWTgmhxePEw5ueSfjnxhMfnqRSHvgfHLXQektil3fPL3Z0SUvHu2h6pLB3B7YDwaLWizjHNS/f1G5g9SFMt4CWlxyu8IdhW+Jt4gUpZw8JJnPpiuarrtOa9v1KbHRW3lw8eAWbvZIgIOcfRCpiK1/sp1S0MiQkKINLh6hy47fecDPglJ5fivjKV0qTear550LZBdJxwJyx26o8grj2jW0ldNNa+Zz5c4XWs4F5970JSGTNXjKVF1NLpneYyNIA1JX/JAWnMdAzeZIi33Gu3LB5GDxquX4rEGvMDcqL5ZK50It9YcEeLJbrFPD68YEf6sgqIhqvjeemNAdWD0ySPt4wSodL60hdDI2IqSLWacKPO854YPdM3icg6bgi8d1XXGwZWtTj5ycQ/nq6K8=; Domain=.network-policy.cloud.ibm.com; Path=/; Expires=Fri, 29 Mar 2024 00:20:12 GMT; Max-Age=7200; HttpOnly
Strict-Transport-Security: max-age=31536000; includeSubDomains
Transaction-Id: 4ed26f8b-658b-4676-8c30-aabca0a75ef8
X-Content-Type-Options: nosniff
X-Correlation-Id: 4ed26f8b-658b-4676-8c30-aabca0a75ef8
X-Proxy-Upstream-Service-Time: 91
X-Ratelimit-Limit: 10
X-Ratelimit-Remaining: 9
X-Ratelimit-Reset: 1711664413

{"description":"test rule config basic","resources":[{"attributes":[{"name":"accountId","value":"37cb83958369439db2ef3d6156f82b9d"},{"name":"serviceName","value":"user-management"}],"tags":[{"name":"tag_name","value":"tag_value"}]}],"contexts":[{"attributes":[{"name":"networkZoneId","value":"3cbe2e308ead56b8021b8a8618089776"}]}],"operations":{"api_types":[{"api_type_id":"crn:v1:bluemix:public:context-based-restrictions::::api-type:","display_name":"All","description":"Protects all service APIs."}]},"enforcement_mode":"disabled","id":"d38463f844a13ed98eb97c48f909cb5a","crn":"crn:v1:bluemix:public:context-based-restrictions:global:a/37cb83958369439db2ef3d6156f82b9d::rule:d38463f844a13ed98eb97c48f909cb5a","href":"https://testing-1-ap-south.network-policy.cloud.ibm.com/v1/rules/d38463f844a13ed98eb97c48f909cb5a","created_at":"2024-03-28T22:19:51Z","created_by_id":"IBMid-550003PATQ","last_modified_at":"2024-03-28T22:19:51Z","last_modified_by_id":"IBMid-550003PATQ"}: timestamp=2024-03-28T17:20:12.810-0500
2024-03-28T17:20:12.811-0500 [INFO]  provider.terraform-provider-ibm: 2024/03/28 17:20:12 [Debug] Request:
GET /v1/rules/d38463f844a13ed98eb97c48f909cb5a HTTP/1.1
Host: testing-1-ap-south.network-policy.cloud.ibm.com
User-Agent: platform-services-go-sdk/0.61.2 (lang=go; arch=arm64; os=darwin; go.version=go1.21.6)
Accept: application/json
Authorization: [redacted]
X-Original-User-Agent: terraform-provider-ibm/1.64.0-beta0
Accept-Encoding: gzip
: timestamp=2024-03-28T17:20:12.811-0500
2024-03-28T17:20:12.811-0500 [INFO]  provider.terraform-provider-ibm: 2024/03/28 17:20:12 [DEBUG] GET https://testing-1-ap-south.network-policy.cloud.ibm.com/v1/rules/d38463f844a13ed98eb97c48f909cb5a: timestamp=2024-03-28T17:20:12.811-0500
2024-03-28T17:20:13.333-0500 [INFO]  provider.terraform-provider-ibm: 2024/03/28 17:20:13 [Debug] Response:
HTTP/2.0 200 OK
Content-Length: 973
Akamai-Grn: 0.b8764017.1711664412.1a12088
Cache-Control: max-age=0, no-cache, no-store
Content-Type: application/json; charset=utf-8
Date: Thu, 28 Mar 2024 22:20:13 GMT
Etag: 1-e7fe6ccc1aa9b260cf9a75f5545918a6
Expires: Thu, 28 Mar 2024 22:20:13 GMT
Pragma: no-cache
Set-Cookie: ak_bmsc=57DD90F650B835369E48F77FAEB2209C~000000000000000000000000000000~YAAQuHZAF1D5bIWOAQAAOjomhxest+gYrOohHxPLLJ83wAD9MzG8dUVxhgdKHMc3nlUQUU8NEYV9NDYf1/GotFqwYBMCldU3hN36WPunj/nkexg3SAiBV9hW9LCedE6shpGNOGvHuIQaAylZebiHhDKzGVfT73oyPasa/latgu49ImB9I3dYwrFfmmaAHNJB+ABx7g/JNjZ3V0tkunACjGG3tXgzBGwTi0MX/jREZdrmgZpqTW3OLo3iZr3eZcnxyI9c4kxO13Xv+h3Mm6kTfHcC5/BzXYyV0UMzP/fWq4+jfn0Z+X9JjSSAJGvWLr4Cr6Jkb6MdifKUH+PdX4uM7RQPf1mzhuGqpuDYmz0rbRwOT0nNRFFhdWZp1XuDLp0VeXSQyE3xnP+/AvxdM1yxppEPNhF4RGUWKww=; Domain=.network-policy.cloud.ibm.com; Path=/; Expires=Fri, 29 Mar 2024 00:20:12 GMT; Max-Age=7199; HttpOnly
Strict-Transport-Security: max-age=31536000; includeSubDomains
Transaction-Id: 9bbf37ef-3261-4dc2-90ed-991ccb535306
X-Content-Type-Options: nosniff
X-Correlation-Id: 9bbf37ef-3261-4dc2-90ed-991ccb535306
X-Proxy-Upstream-Service-Time: 54
X-Ratelimit-Limit: 10
X-Ratelimit-Remaining: 8
X-Ratelimit-Reset: 1711664413

{"description":"test rule config basic","resources":[{"attributes":[{"name":"accountId","value":"37cb83958369439db2ef3d6156f82b9d"},{"name":"serviceName","value":"user-management"}],"tags":[{"name":"tag_name","value":"tag_value"}]}],"contexts":[{"attributes":[{"name":"networkZoneId","value":"3cbe2e308ead56b8021b8a8618089776"}]}],"operations":{"api_types":[{"api_type_id":"crn:v1:bluemix:public:context-based-restrictions::::api-type:","display_name":"All","description":"Protects all service APIs."}]},"enforcement_mode":"disabled","id":"d38463f844a13ed98eb97c48f909cb5a","crn":"crn:v1:bluemix:public:context-based-restrictions:global:a/37cb83958369439db2ef3d6156f82b9d::rule:d38463f844a13ed98eb97c48f909cb5a","href":"https://testing-1-ap-south.network-policy.cloud.ibm.com/v1/rules/d38463f844a13ed98eb97c48f909cb5a","created_at":"2024-03-28T22:19:51Z","created_by_id":"IBMid-550003PATQ","last_modified_at":"2024-03-28T22:19:51Z","last_modified_by_id":"IBMid-550003PATQ"}: timestamp=2024-03-28T17:20:13.332-0500
2024-03-28T17:20:13.335-0500 [WARN]  Provider "provider[\"github.ibm.com/ibm-cloud/ibm\"]" produced an unexpected new value for ibm_cbr_rule.cbr_rule, but we are tolerating it because it is using the legacy plugin SDK.
    The following problems may be the cause of any confusing errors from downstream operations:
      - .transaction_id: was null, but now cty.StringVal("")
      - .x_correlation_id: was null, but now cty.StringVal("")
      - .resources[0].tags[0].operator: was null, but now cty.StringVal("")
ibm_cbr_rule.cbr_rule: Creation complete after 22s [id=d38463f844a13ed98eb97c48f909cb5a]

Apply complete! Resources: 2 added, 0 changed, 0 destroyed.

@tyao117 tyao117 marked this pull request as ready for review March 28, 2024 22:41
@ocofaigh
Copy link
Contributor

@zhenwan you might want to review?

if response != nil && response.StatusCode == 404 {
d.SetId("")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Chane the retry logic to use Resource.RetryContext

err = resource.RetryContext(context, 5*time.Second, func() *resource.RetryError {

Copy link
Contributor Author

@tyao117 tyao117 Apr 1, 2024

Choose a reason for hiding this comment

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

getZoneOptions.SetZoneID(d.Id())

zone, response, err := contextBasedRestrictionsClient.GetZoneWithContext(context, getZoneOptions)
if err != nil {
if response != nil && response.StatusCode == 404 {
d.SetId("")
return nil
// Manual change. leverage retry for the read due to eventual consistency of the service.
Copy link
Collaborator

Choose a reason for hiding this comment

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

similar comment

Copy link
Contributor Author

@tyao117 tyao117 Apr 1, 2024

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can use retry.RetryContext instead of Resource.RetryContext as sugested in deprecation

Copy link
Contributor Author

@tyao117 tyao117 Apr 2, 2024

Choose a reason for hiding this comment

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

@zhenwan
Copy link
Contributor

zhenwan commented Apr 1, 2024

@ocofaigh yes

getRuleOptions := &contextbasedrestrictionsv1.GetRuleOptions{}
getRuleOptions.SetRuleID(id)
_, response, err := cbrClient.GetRuleWithContext(context, getRuleOptions)
if err != nil && response.StatusCode == 404 {
Copy link
Contributor

@zhenwan zhenwan Apr 3, 2024

Choose a reason for hiding this comment

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

better to have log output with error and waiting time for next try if the retry does not do it

if response != nil && response.StatusCode == 404 {
d.SetId("")
return nil
log.Printf("[INFO] Read cbr rule response status code: 404, provider will try again. %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

add sleeping time before call waitForCbrRuleRead ?

getZoneOptions := &contextbasedrestrictionsv1.GetZoneOptions{}
getZoneOptions.SetZoneID(id)
_, response, err := cbrClient.GetZoneWithContext(context, getZoneOptions)
if err != nil && response.StatusCode == 404 {
Copy link
Contributor

Choose a reason for hiding this comment

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

better to have log output with error and waiting time for next try if the retry does not do it

d.SetId("")
return nil
// Manual change. leverage retry for the read due to eventual consistency of the service.
log.Printf("[INFO] Read cbr zone response status code: 404, provider will try again. %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

add sleeping time before call waitForCbrZoneRead ?

// end Manual Change
} else {
log.Printf("[DEBUG] GetRuleWithContext failed %s\n%s", err, response)
return diag.FromErr(fmt.Errorf("GetRuleWithContext failed %s\n%s", err, response))
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This logic handle the case when CBR rule is deleted outside Terraform context and when user runs plan it should diff a new resource will be created instead of throwing error
Move this wait logic here after creation wait till its acauires desired state

Copy link
Contributor

@zhenwan zhenwan Apr 21, 2024

Choose a reason for hiding this comment

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

@hkantare I don't agree with your comment above. The code you highlighted above is already in the master branch which is utilized to manage rule read errors if the StatusCode is not 404

The logic Tim updated to the function resourceIBMCbrRuleRead is to handle the Eventual consistency issue that a user reported here. The Eventual Consistency issue arises because the rule was created in one region, but the read call for the rule went to another region before the rule data became available in the second region.
Tim updated
if response != nil && response.StatusCode == 404 { } code block is updated by adding some retry logic if the rule can not be found after run terraform apply (404 error), we need to use the code you highlighted to handle other errors.

Please let me know if I miss understood your comment.

@hkantare
Copy link
Collaborator

@zhenwan I understood we added this logic to support Eventual but we are breaking the scenario which currently exists in master branch

Steps

  1. Create a CBR rule via terraform
  2. delete CBR using ui/cli/api
    3)Rerun terraform plan it shd show diff (resource needs to be reprovisioned)
    Can you share the result for above scenario

@zhenwan
Copy link
Contributor

zhenwan commented Apr 22, 2024

@hkantare ok, Now I understand what you meant. I will update the code.
Here is the result by following your steps

ibm-context-based-restrictions % terraform plan

ibm_cbr_zone.cbr_zone_instance: Refreshing state... [id=2a5eaec9f4f55b8fc4e76dcb7e9167c6]
data.ibm_cbr_zone.cbr_zone_instance: Reading...
ibm_cbr_rule.cbr_rule_instance: Refreshing state... [id=2a5eaec9f4f55b8fc4e76dcb7e916a3c]
data.ibm_cbr_zone.cbr_zone_instance: Read complete after 0s [id=2a5eaec9f4f55b8fc4e76dcb7e9167c6]

Planning failed. Terraform encountered an error while generating this plan.

╷
│ Error: GetRuleWithContext failed context deadline exceeded
│ {
│     "StatusCode": 404,
│     "Headers": {
│         "Akamai-Grn": [
│             "0.4d155d68.1713802380.4e733676"
│         ],
│         "Cache-Control": [
│             "no-cache,no-store"
│         ],
│         "Connection": [
│             "keep-alive"
│         ],
│         "Content-Length": [
│             "281"
│         ],
│         "Content-Type": [
│             "application/json; charset=utf-8"
│         ],
│         "Date": [
│             "Mon, 22 Apr 2024 16:13:00 GMT"
│         ],
│         "Expires": [
│             "Thursday, 1 January 1970 00:00:00 GMT"
│         ],
│         "Pragma": [
│             "no-cache"
│         ],
│         "Strict-Transport-Security": [
│             "max-age=31536000; includeSubDomains"
│         ],
│         "Transaction-Id": [
│             "1b1fa887-3be7-4576-b582-d7a6c5c3b521"
│         ],
│         "X-Content-Type-Options": [
│             "nosniff"
│         ],
│         "X-Correlation-Id": [
│             "1b1fa887-3be7-4576-b582-d7a6c5c3b521"
│         ],
│         "X-Proxy-Upstream-Service-Time": [
│             "17"
│         ],
│         "X-Ratelimit-Limit": [
│             "10"
│         ],
│         "X-Ratelimit-Remaining": [
│             "9"
│         ],
│         "X-Ratelimit-Reset": [
│             "1713802381"
│         ]
│     },
│     "Result": {
│         "errors": [
│             {
│                 "code": "rule_not_found_in_store",
│                 "message": "The rule '2a5eaec9f4f55b8fc4e76dcb7e916a3c' is not found in store.",
│                 "target": {
│                     "name": "rule_id",
│                     "type": "parameter",
│                     "value": "2a5eaec9f4f55b8fc4e76dcb7e916a3c"
│                 }
│             }
│         ],
│         "status_code": 404,
│         "trace": "1b1fa887-3be7-4576-b582-d7a6c5c3b521"
│     },
│     "RawResult": null
│ }
│ 
│ 
│   with ibm_cbr_rule.cbr_rule_instance,
│   on main.tf line 25, in resource "ibm_cbr_rule" "cbr_rule_instance":
│   25: resource "ibm_cbr_rule" "cbr_rule_instance" {
│ 
╵

@zhenwan
Copy link
Contributor

zhenwan commented Apr 22, 2024

@hkantare code has been updated, please re-review.

@hkantare
Copy link
Collaborator

Thanks
Can we resolve conflits ibm/acctest/acctest.go

@zhenwan
Copy link
Contributor

zhenwan commented Apr 23, 2024

@hkantare Resolved the conflicts.

@hkantare hkantare merged commit 89015e4 into IBM-Cloud:master Apr 23, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants