Addition of new generated resource and data source for pkiTP #1145

shrsr · 2024-01-23T14:25:49Z

No description provided.

docs/data-sources/certificate_authority.md

docs/resources/annotation.md

docs/resources/certificate_authority.md

internal/provider/resource_aci_certificate_authority.go

docs/data-sources/certificate_authority.md

docs/data-sources/key_ring.md

internal/provider/provider.go

internal/provider/resource_aci_annotation.go

internal/provider/resource_aci_certificate_authority.go

internal/provider/resource_aci_rest_managed_test.go

gen/definitions/classes.yaml

gen/templates/testvars.yaml.tmpl

examples/data-sources/aci_certificate_authority/data-source.tf

examples/data-sources/aci_key_ring/data-source.tf

examples/resources/aci_certificate_authority/resource-all-attributes.tf

gen/templates/resource_example.tf.tmpl

gen/templates/resource_example_all_attributes.tf.tmpl

go.mod

samiib

When you get a chance can you please also rebase before regenerating again.

gen/definitions/properties.yaml

gen/generator.go

gen/templates/datasource.md.tmpl

gen/templates/testvars.yaml.tmpl

gen/generator.go

internal/provider/resource_aci_certificate_authority_test.go

anvitha-jain

LGTM

gmicol

LGTM

sajagana

LGMT!

internal/provider/resource_aci_certificate_authority.go

gmicol

Can you comment your new functions in the generator? Would be very helpful for future reference.

gen/definitions/classes.yaml

gen/templates/datasource_test.go.tmpl

gen/templates/resource_test.go.tmpl

sajagana

LGTM!

codecov-commenter · 2024-06-10T04:24:30Z

Codecov Report

Attention: Patch coverage is 75.77497% with 422 lines in your changes missing coverage. Please review.

Project coverage is 85.52%. Comparing base (6f45104) to head (936e2a3).

Files	Patch %	Lines
internal/provider/resource_aci_key_ring.go	81.23%	70 Missing and 45 partials ⚠️
...nal/provider/resource_aci_certificate_authority.go	80.94%	60 Missing and 33 partials ⚠️
...rnal/provider/resource_aci_out_of_band_contract.go	41.17%	10 Missing and 10 partials ⚠️
...ernal/provider/resource_aci_pim_route_map_entry.go	41.93%	9 Missing and 9 partials ⚠️
...rnal/provider/resource_aci_l3out_consumer_label.go	42.85%	8 Missing and 8 partials ⚠️
...rnal/provider/resource_aci_l3out_provider_label.go	44.00%	7 Missing and 7 partials ⚠️
internal/provider/resource_aci_endpoint_tag_ip.go	45.45%	6 Missing and 6 partials ⚠️
internal/provider/resource_aci_endpoint_tag_mac.go	45.45%	6 Missing and 6 partials ⚠️
...al/provider/resource_aci_l3out_node_sid_profile.go	45.45%	6 Missing and 6 partials ⚠️
...al/provider/resource_aci_netflow_monitor_policy.go	53.84%	6 Missing and 6 partials ⚠️
... and 14 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1145      +/-   ##
==========================================
- Coverage   87.03%   85.52%   -1.52%     
==========================================
  Files          43       47       +4     
  Lines       11223    12965    +1742     
==========================================
+ Hits         9768    11088    +1320     
- Misses        991     1233     +242     
- Partials      464      644     +180

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

samiib

I fixed up the user config re-ran the CI tests and noticed that some are failing in APIC v5.2.
TestAccDataSourcePkiKeyRingWithDefault & TestAccResourcePkiKeyRingWithDefault
I suspect because the properties the error is referring to are only in v6.0+

If so, is it possible to add a new option to check version in testAccPreCheck and skip if the APIC version is not supported? Similar to cloud/apic. I think that would be okay for now instead of having multiple tests with a subset of properties in older versions which would be complicated to generate.

If this PR is already too big we can do the simple version check in a separate PR.

Also, do we have a cloud APIC that I can add to this CI that is in a good state? Just so we can run the cloud tests instead of always skipping in the CI.

shrsr · 2024-06-10T17:39:28Z

@samiib Thanks for running the CI test for this PR. It's a big relief that the test is passing on v6.0

Yeah, I am aware of this issue. When I introduced the test_type functionality we still didn't have a CI in place for TF resource testing that could run tests on multiple versions of APIC.

I agree. At this juncture the easy and practical solution is to skip the entire test for a resource on an older version of APIC where the test contains at least one property that is only supported on the latest version of APIC.
We need to make a few changes in the generator and the test template for this.

Yes, we have a cloud APIC but I'm not sure if we should test resources against the cAPIC since it has reached its EoL.

sajagana

LGTM!

samiib

LGTM! Thank you for adding the version check.

docs/data-sources/netflow_monitor_policy.md

akinross · 2024-06-14T06:17:19Z

docs/resources/certificate_authority.md

+// This example is only applicable to Cisco Cloud Network Controller
+resource "aci_certificate_authority" "example_tenant" {
+  parent_dn         = aci_tenant.example.id
+  certificate_chain = <<EOT


did you consider doing the second example with a file input example? https://developer.hashicorp.com/terraform/language/functions/file

docs/resources/certificate_authority.md

gen/generator.go

internal/provider/test_constants.go

akinross · 2024-06-14T06:35:46Z

internal/provider/test_constants.go

+
+const testConfigDefaultMin = ``
+
+const testConfigFvTenantMinDependencyWithPkiTP = testConfigFvTenantMin + `


why are these defined as constants? and not generated?

akinross · 2024-06-14T06:41:56Z

please also rebase once #1236 is merged

… pkiTP

shrsr self-assigned this Jan 23, 2024

This was linked to issues Jan 23, 2024

APIC Certificate Import Support - #1072

Open

Create a resource to configure "Certificate Authority - pkiTP" object for the Azure CNC #1134

Open

Create a resource to configure "Key Ring - pkiKeyRing" object for the Azure CNC #1135

Open

shrsr force-pushed the key branch from b2f52a6 to 960ba96 Compare March 4, 2024 20:54

shrsr requested review from akinross, samiib, lhercot, anvitha-jain, sajagana and gmicol March 7, 2024 17:35

akinross requested changes Mar 8, 2024

View reviewed changes

shrsr removed a link to an issue Mar 12, 2024

APIC Certificate Import Support - #1072

Open

shrsr mentioned this pull request Mar 12, 2024

APIC Certificate Import Support - #1072

Open

shrsr requested a review from akinross March 16, 2024 21:22

akinross requested changes Mar 20, 2024

View reviewed changes

shrsr force-pushed the key branch from e7c9c61 to 8f214c6 Compare March 27, 2024 19:51

shrsr requested a review from akinross March 27, 2024 19:58

lhercot requested changes Mar 27, 2024

View reviewed changes

samiib requested changes Mar 28, 2024

View reviewed changes

shrsr requested a review from lhercot March 28, 2024 13:31

sajagana requested changes Mar 28, 2024

View reviewed changes

gen/generator.go Show resolved Hide resolved

internal/provider/resource_aci_certificate_authority_test.go Outdated Show resolved Hide resolved

shrsr requested review from sajagana and samiib March 29, 2024 15:19

anvitha-jain previously approved these changes Mar 29, 2024

View reviewed changes

gmicol previously approved these changes Mar 29, 2024

View reviewed changes

sajagana previously approved these changes Apr 1, 2024

View reviewed changes

shrsr dismissed stale reviews from sajagana, gmicol, and anvitha-jain via fbb66bb April 2, 2024 16:21

shrsr dismissed stale reviews from sajagana and gmicol via cfdeeaa April 23, 2024 10:56

shrsr force-pushed the key branch 2 times, most recently from bae14c2 to d66746d Compare April 24, 2024 15:50

shrsr requested review from akinross and sajagana April 25, 2024 15:31

akinross reviewed Apr 29, 2024

View reviewed changes

internal/provider/resource_aci_certificate_authority.go Show resolved Hide resolved

gmicol requested changes May 3, 2024

View reviewed changes

shrsr force-pushed the key branch from e75c0a8 to b72841c Compare May 23, 2024 16:38

shrsr requested review from akinross and gmicol May 30, 2024 18:16

sajagana requested changes Jun 5, 2024

View reviewed changes

shrsr requested a review from sajagana June 7, 2024 12:52

sajagana previously approved these changes Jun 7, 2024

View reviewed changes

samiib requested changes Jun 10, 2024

View reviewed changes

shrsr dismissed sajagana’s stale review via 9f97fd7 June 11, 2024 17:51

shrsr requested review from sajagana and samiib June 11, 2024 17:52

sajagana previously approved these changes Jun 12, 2024

View reviewed changes

samiib previously approved these changes Jun 12, 2024

View reviewed changes

anvitha-jain previously approved these changes Jun 12, 2024

View reviewed changes

akinross requested changes Jun 14, 2024

View reviewed changes

shrsr dismissed stale reviews from anvitha-jain, samiib, and sajagana via f540fe4 June 14, 2024 19:06

[minor_change] Addition of new generated resource and data source for…

dceba6c

… pkiTP

shrsr force-pushed the key branch from f540fe4 to dceba6c Compare July 16, 2024 12:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Addition of new generated resource and data source for pkiTP #1145

Addition of new generated resource and data source for pkiTP #1145

shrsr commented Jan 23, 2024

samiib left a comment

anvitha-jain left a comment

gmicol left a comment

sajagana left a comment

gmicol left a comment

sajagana left a comment

codecov-commenter commented Jun 10, 2024

samiib left a comment •

edited

Loading

shrsr commented Jun 10, 2024

sajagana left a comment

samiib left a comment

akinross Jun 14, 2024

akinross Jun 14, 2024

akinross commented Jun 14, 2024


		const testConfigDefaultMin = ``

		const testConfigFvTenantMinDependencyWithPkiTP = testConfigFvTenantMin + `

Addition of new generated resource and data source for pkiTP #1145

Are you sure you want to change the base?

Addition of new generated resource and data source for pkiTP #1145

Conversation

shrsr commented Jan 23, 2024

samiib left a comment

Choose a reason for hiding this comment

anvitha-jain left a comment

Choose a reason for hiding this comment

gmicol left a comment

Choose a reason for hiding this comment

sajagana left a comment

Choose a reason for hiding this comment

gmicol left a comment

Choose a reason for hiding this comment

sajagana left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 10, 2024

Codecov Report

samiib left a comment • edited Loading

Choose a reason for hiding this comment

shrsr commented Jun 10, 2024

sajagana left a comment

Choose a reason for hiding this comment

samiib left a comment

Choose a reason for hiding this comment

akinross Jun 14, 2024

Choose a reason for hiding this comment

akinross Jun 14, 2024

Choose a reason for hiding this comment

akinross commented Jun 14, 2024

samiib left a comment •

edited

Loading