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

Terraformer phase 2 updates #921

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

RutvikS-crest
Copy link
Contributor

* Updated Read function in access_switch_policy_group

* User security role update (#151)

* Updated fabricNodeControl and userSecurityDomainRole

* Updated user security domain

* Updated Read Function for UI Deletion Test case (#152)

* added set for annotation in ldap_group_map

* Updated resources (#155)

* aaep_to_domain resource file updated, dn changed and description removed (#156)

* updated documentation

* updated documentation for match_rule

* resource_tagannotation updated (#159)

* tag_resource schema updated

* Updated Index file of Interface Blacklist

* Updated Mgmt Zone (#162)

* updated docs
@RutvikS-crest RutvikS-crest changed the title Terraformer updates (#920) Terraformer phase 2 updates Aug 5, 2022
@@ -24,12 +24,20 @@ func resourceAciContractInterfaceRelationship() *schema.Resource {
},

SchemaVersion: 1,
Schema: AppendBaseAttrSchema(map[string]*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.

Why do we remove this function and add the annotation as separate entries to the Schema?

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 Function appends description and annotation attribute to the respective resource schema. Some of the ACI classes doesn't support description but may support annotation, Hence addition of annotation separately.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you consider creating separate function for annotation only and re-use this? Also have a look at the AppendAttrSchemas function in https://github.com/CiscoDevNet/terraform-provider-aci/blob/master/aci/base_attr_schema.go, this could perhaps be leveraged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a GetAnnotation function for uniform schema implementation.

@lhercot
Copy link
Member

lhercot commented Aug 19, 2022

I see vendor file updated but not a new version of the aci-go-client. Was this already merged into go-client?

@RutvikS-crest
Copy link
Contributor Author

@lhercot, Yes as it was a minor change, it is already merged to main branch

Comment on lines 28 to 35
Schema: map[string]*schema.Schema{
"annotation": {
Type: schema.TypeString,
Optional: true,
Computed: true,
DefaultFunc: func() (interface{}, error) {
return "orchestrator:terraform", nil
},
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as Akini on other file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a GetAnnotation function for uniform schema implementation.

Comment on lines 32 to 38
"annotation": &schema.Schema{
Type: schema.TypeString,
Optional: true,
Computed: true,
DefaultFunc: func() (interface{}, error) {
return "orchestrator:terraform", nil
},
Copy link
Member

Choose a reason for hiding this comment

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

same comment as Akini on other file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a GetAnnotation function for uniform schema implementation.

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.

None yet

3 participants