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.

Sign up for GitHub

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

Jump to bottom

CBR: context-based-restriction update for Enforcement mode support #3853

Merged
merged 14 commits into from
Jun 30, 2022
Merged

CBR: context-based-restriction update for Enforcement mode support #3853

merged 14 commits into from
Jun 30, 2022

Conversation

zhenwan
Copy link
Contributor

@zhenwan zhenwan commented Jun 18, 2022 •
edited
Loading

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

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'

...

image

@zhenwan
Copy link
Contributor Author

zhenwan commented Jun 18, 2022

Please hold merging until PR IBM/platform-services-go-sdk#191 get merged

@zhenwan
Copy link
Contributor Author

zhenwan commented Jun 21, 2022

Can someone review this PR?

hkantare
hkantare reviewed Jun 24, 2022
View reviewed changes
ibm/service/contextbasedrestrictions/data_source_ibm_cbr_rule.go Outdated
Type: schema.TypeString,
Required: true,
Description: "The ID of a rule.",
},
"crn": {
"id": &schema.Schema{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't add "id" to the schema ..We have id for each resoure/datasource from Terraform core

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

ibm/service/contextbasedrestrictions/data_source_ibm_cbr_rule.go
@@ -18,42 +18,47 @@ import (

func DataSourceIBMCbrRule() *schema.Resource {
return &schema.Resource{
ReadContext: dataSourceIBMCbrRuleRead,
Copy link
Collaborator

Choose a reason for hiding this comment

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

why were the names changed to Captial letter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are auto-generated code from new code generator openapi-sdkgen.sh
can you please confirm with generator team to see if Capital letter names are correct.
I believe the code will be broken if I change them to lower cases.

ibm/service/contextbasedrestrictions/resource_ibm_cbr_rule.go Outdated
@@ -157,7 +176,7 @@ func ResourceIBMCbrRule() *schema.Resource {
}

func ResourceIBMCbrRuleValidator() *validate.ResourceValidator {
validateSchema := make([]validate.ValidateSchema, 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you make it 0nthe inital length...its a generator issue we posted to them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

change it to:
validateSchema := make([]validate.ValidateSchema, 0)

ibm/service/contextbasedrestrictions/resource_ibm_cbr_zone.go
Importer: &schema.ResourceImporter{},

Schema: map[string]*schema.Schema{
"name": {
"name": &schema.Schema{
Type: schema.TypeString,
Copy link
Collaborator

Choose a reason for hiding this comment

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

why some of required were changed to optional ...Are they really optional (in all resources i have seen it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

made necessary changes

ibm/service/contextbasedrestrictions/resource_ibm_cbr_zone.go
Type: schema.TypeString,
Computed: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

account_id is computed or required

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is required in "ref" schema

ibm/service/contextbasedrestrictions/resource_ibm_cbr_zone.go
)

resourceValidator := validate.ResourceValidator{ResourceName: "ibm_cbr_zone", Schema: validateSchema}
return &resourceValidator
}

func getIBMCbrAccountId(meta interface{}) (string, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why we removed this ? We added accoutn_id as optional...will this work if user doens't send account_id

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is removed by code generator.
This function is not called by nowhere

website/docs/d/cbr_rule.html.markdown
@@ -2,7 +2,7 @@
layout: "ibm"
page_title: "IBM : ibm_cbr_rule"
description: |-
Get information about cbr_rule
Copy link
Collaborator

Choose a reason for hiding this comment

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

i thnk we shd have one tab here else subcategory will not be applicable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

website/docs/d/cbr_rule.html.markdown Outdated
* Constraints: The maximum length is `64` characters. The minimum length is `1` character. The value must match regular expression `^[a-zA-Z0-9]+$`.
* `value` - (String) The tag attribute value.
* Constraints: The maximum length is `1000` characters. The minimum length is `1` character. The value must match regular expression `^[a-zA-Z0-9 _*?.-]+$`.
* `contexts` - (Required, List) The contexts this rule applies to.
Copy link
Collaborator

Choose a reason for hiding this comment

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

A computed attributes under Attribute Reference can't have Required/optiona

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

website/docs/r/cbr_rule.html.markdown Outdated

* `version` - Version of the cbr_rule.

## Provider Configuration
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this section from docs ..we rasied an issue with generator team

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@zhenwan
Copy link
Contributor Author

zhenwan commented Jun 27, 2022

@hkantare Can you please review this PR, I updated the code based on your reviews

@hkantare hkantare merged commit 5e52061 into IBM-Cloud:master Jun 30, 2022
hkantare pushed a commit that referenced this pull request Jul 1, 2022
@zhenwan @hkantare
CBR: context-based-restriction update for Enforcement mode support (#…
722609f 
…3853)

* CBR: Terraform Provider update for rule Enforcement-mode support

Terraform: Rule Enforcement Mode Support
https://github.ibm.com/IAM/cbr-issues/issues/113

* secrets.baseline update

* fix import

* update secrets.baseline

* update platform-service-so-sdk version

* Update .secrets.baseline

* updated on PR review comments

* Update cbr_zone.html.markdown
SunithaGudisagarIBM pushed a commit to ibm-vpc/terraform-provider-ibm that referenced this pull request Sep 14, 2022
@zhenwan @SunithaGudisagarIBM1
CBR: context-based-restriction update for Enforcement mode support (I…
62c02da 
…BM-Cloud#3853)

* CBR: Terraform Provider update for rule Enforcement-mode support

Terraform: Rule Enforcement Mode Support
https://github.ibm.com/IAM/cbr-issues/issues/113

* secrets.baseline update

* fix import

* update secrets.baseline

* update platform-service-so-sdk version

* Update .secrets.baseline

* updated on PR review comments

* Update cbr_zone.html.markdown
SunithaGudisagarIBM pushed a commit to ibm-vpc/terraform-provider-ibm that referenced this pull request Sep 14, 2022
@zhenwan @SunithaGudisagarIBM1
CBR: context-based-restriction update for Enforcement mode support (I…
e525a53 
…BM-Cloud#3853)

* CBR: Terraform Provider update for rule Enforcement-mode support

Terraform: Rule Enforcement Mode Support
https://github.ibm.com/IAM/cbr-issues/issues/113

* secrets.baseline update

* fix import

* update secrets.baseline

* update platform-service-so-sdk version

* Update .secrets.baseline

* updated on PR review comments

* Update cbr_zone.html.markdown
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hkantare hkantare hkantare left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zhenwan @hkantare