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

Adding changes for replacement of key_protect parameter and adding the fix for 3394 #4618

Merged

Conversation

IBM-diksha
Copy link
Collaborator

@IBM-diksha IBM-diksha commented Jun 8, 2023

Test results :

--- PASS: TestAccIBMCOSKPKmsParamValid (84.76s)
PASS
ok github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/cos 86.384s
--- PASS: TestAccIBMCOSKPKmsParamWithInvalidCRN (49.70s)
PASS
ok github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/cos 51.390s

--- PASS: TestAccIBMCOSHPCSKmsParamWithInvalidCRN (44.35s)
PASS
ok github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/cos 114.080s
--- PASS: TestAccIBMCOSHPCSKmsParamWithInvalidCRN (44.35s)
PASS

ok github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/cos (cached)

--- PASS: TestAccIBMCOSKMSBothParamProvided (0.34s)
PASS
ok github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/cos 1.835s

KMS test cases
=== RUN TestAccIBMKMSResource_basic
--- PASS: TestAccIBMKMSResource_basic (242.06s)
PASS
ok github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/kms 244.089s

@IBM-diksha IBM-diksha changed the title Adding changes for replacement of key_protect parameter Adding changes for replacement of key_protect parameter and adding the fix for 3394 Jun 8, 2023
@william8siew
Copy link
Contributor

https://github.com/IBM-Cloud/terraform-provider-ibm/tree/master/examples/ibm-kms
You missed this file

@william8siew
Copy link
Contributor

And this
https://github.com/IBM-Cloud/terraform-provider-ibm/blob/master/examples/ibm-kms/main.tf

@william8siew
Copy link
Contributor

https://github.com/IBM-Cloud/terraform-provider-ibm/blob/master/examples/ibm-key-protect/README.md

@william8siew
Copy link
Contributor

https://github.com/IBM-Cloud/terraform-provider-ibm/blob/master/examples/ibm-key-protect/main.tf

@william8siew
Copy link
Contributor

https://github.com/IBM-Cloud/terraform-provider-ibm/blob/master/ibm/service/kms/resource_ibm_kms_key_test.go

@william8siew
Copy link
Contributor

All the files I have linked above still make reference to

resource "ibm_cos_bucket"

using
key_protect and not kms_key_crn. Please fix this.
You should also be running the key protect/kms test suites

Copy link
Contributor

@william8siew william8siew left a comment

Choose a reason for hiding this comment

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

lgtm

} else {
d.Set("kms_key_crn", head.IBMSSEKPCrkId)
}
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we seeting empty ?

Description: "CRN of the key you want to use data at rest encryption",
Type: schema.TypeString,
ForceNew: true,
Optional: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to mark this as deprecated

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As of now we are not deprecating the key_protect parameter.
we will just be recommending new customers to use the new parameter.

@hkantare hkantare merged commit 946b4bf into IBM-Cloud:master Jun 20, 2023
omaraibrahim pushed a commit to omaraibrahim/terraform-provider-ibm that referenced this pull request Jul 20, 2023
…e fix for 3394 (IBM-Cloud#4618)

* Adding the fix for issue 3394 and 1947

* Adding documentation for kms resource

* Changes to kms example ,readme and test

* Addressing the PR comment for code formatting

* removing unnecessary changes from datasource
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.

3 participants