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

fix(tls_subscription): ensure configuration_id is current value (not initial) #824

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

virgofx
Copy link
Contributor

@virgofx virgofx commented Mar 27, 2024

The TLS configuration ID returned by the API is the initial TLS configuration when first activated. Instead, this value should represent the current TLS configuration id which can be found via the following API call.

https://api.fastly.com/tls/domains?filter[tls_certificates.id]={cert-id}&include=tls_activations

@virgofx virgofx force-pushed the mark/fix-787 branch 3 times, most recently from 403c753 to a1e4936 Compare March 28, 2024 15:32
@virgofx
Copy link
Contributor Author

virgofx commented Apr 5, 2024

@Integralist Any chance we could get this reviewed and hopefully merged in a new release? Thanks!

@Integralist
Copy link
Collaborator

Thanks @virgofx for your patience.

I'm reviewing this today.

Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

LGTM. I'm not going to merge immediately, as I would like a member of the Fastly TLS team to take a pass over my review just to sanity check some of my understanding around activation objects.

fastly/resource_fastly_tls_subscription.go Outdated Show resolved Hide resolved
fastly/resource_fastly_tls_subscription.go Show resolved Hide resolved
fastly/resource_fastly_tls_subscription.go Outdated Show resolved Hide resolved
fastly/resource_fastly_tls_subscription.go Outdated Show resolved Hide resolved
@virgofx
Copy link
Contributor Author

virgofx commented Apr 8, 2024

Thanks for the review. All suggestions look great, updating...

@virgofx
Copy link
Contributor Author

virgofx commented Apr 8, 2024

Latest changes incorporated. Additionally, re-tested locally and confirmed the sort was specified correctly.

@virgofx virgofx force-pushed the mark/fix-787 branch 2 times, most recently from 9793507 to fb9e1d0 Compare April 8, 2024 17:30
@Integralist Integralist merged commit 0e020c2 into fastly:main Apr 9, 2024
9 checks passed
@Integralist
Copy link
Collaborator

Thanks @virgofx for your work on this PR.

I'm not going to publish a new release just yet as I have a couple of other TF tasks I'm looking into at the moment. But hopefully very soon.

@virgofx virgofx deleted the mark/fix-787 branch April 17, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fastly TLS Subscription Read doesn't use the Current Value (Uses original)
2 participants