Custom TLS API

[joel-g]

My organization needs Custom TLS certs and we hope to not maintain our own fork.

I may have discovered a platform bug. I have noticed that POST /tls/certificates will return a 500 Internal Server Error when the "type" field is blank (see https://developer.fastly.com/reference/api/tls/custom-certs/certificates/)

The Private Key (POST /tls/private_keys) and Platform TLS (POST /tls/bulk/certificates) APIs do not return an error if "type" is blank.

This is relevant because the jsonapi marshaller only marshals the attributes and not the type. Therefore the CreateCustomCertificate() function in this PR does not currently work. I can work around this by adding "type" to the Go type and marshaling however this will break the convention found throughout the library.

[joel-g]

I have also reported this possible bug in an enterprise support ticket: #300454

[ryansouza]

Hey @joel-g. I believe I've found a fix for getting the correct type in the request json. In my testing adding a primary ID field to CreateCustomCertificateInput did cause the type to be set and did not require the ID to be set nor did it include an extraneous empty ID field in the json.

type CreateCustomCertificateInput struct {
	ID       string `jsonapi:"primary,tls_certificate"`
	CertBlob string `jsonapi:"attr,cert_blob"`
	Name     string `jsonapi:"attr,name"`
}

and from testing

// json before
{"data":{"type":"","attributes":{"cert_blob":"-----BEGIN CERTIFICATE-----\n...\n-----END CERTIFICATE-----\n","name":"My certificate"}}}

// json after
{"data":{"type":"tls_certificate","attributes":{"cert_blob":"-----BEGIN CERTIFICATE-----\n...\n-----END CERTIFICATE-----\n","name":"My certificate"}}}

Does this look like it will fix the issue?

[joel-g]

@ryansouza I actually discovered the same thing this morning and added a "Type string jsonapi:"primary,tls_certificate"" field that achieves the same thing.

I also have pushed more files to cover the /tls/activation and /tls/configuration API.

/tls/activation has the same issue and returns a server error if "type" is not present in the POST/PATCH update. I have added the field to the input struct to work around this.

That solves the blocker on this being mergeable.

I see there is a dev-v2 branch. Should I re-base this PR to that branch or leave it on master?

[phamann]

Hey @joel-g thank you for your contribution, this is a very large API with multiple endpoints, so we really appreciate your effort here. While there are a lot of comments, the majority are minor and just to enforce project convention and the others around how the JSONAPI library behaves. Let us know if you have any questions/issues.

(co-authored with @Integralist)

[joel-g]

@phamann Accepted your suggestions and removed that pesky Type field 👍

[phamann]

LGTM 🎉

Thank you SO MUCH for your contribution @joel-g and for your patience and understanding throughout this process. Apologies it's taken so long. Part of the reason has been we're transitioning the go-fastly project over to a new internal team and owner, @Integralist, which will hopefully mean there won't be as much delay in the future.

Related: I want him to have one final look over the PR and then we will merge and release this week for you.

[joel-g]

@phamann that was actually very fast compared so some vendors open source. And it was a big PR so I'm happy to "get it right". I'm already working on adding these to https://github.com/fastly/terraform-provider-fastly so I'll see you there

[Integralist]

Thanks for the great work on this PR @joel-g 🙂 it's a huge chunk of the TLS API so we really appreciate the effort you've put into this.

Before I approve the PR I just had some small comments for you. If you could take a look and either address/resolve them or let me know your thoughts.

[Integralist]

Hi @joel-g a few quick comments.

[Integralist]

A few more small tweaks required please Joel. Thanks!

[Integralist]

Apologies @joel-g I think some of my suggestions might have been missed/mis-understood. I've added/re-added some stuff.

[Integralist]

OK I think we might have gotten there 🎉

Thanks for all your patience and hard work @joel-g 👍🏻

Note: I have three other Go-Fastly PRs that I'm reviewing today, and so I might not cut a new Go-Fastly release just yet (e.g. once merging this PR) as I'd like to combine the other PR changesets into a single release.