Fixed issue with creating certs for root domains.

n3wt0n / AzureWebAppSSLManager

Acquires and manages free SSL certificates for Azure Web App and Azure Functions applications.

MIT License

76 stars 29 forks source link

Fixed issue with creating certs for root domains. #16

Closed RichMercer closed 4 years ago

RichMercer commented 4 years ago

Removed call causing issues with root domains and also updated docs as this is no longer an issue.

n3wt0n commented 4 years ago

I haven't tried it yet, but I have a question. The current flow is:

Create an order for the cert for a domain
Create a CNAME in Azure DNS to validate the ownership of that domain
Obtain the cert
Install it on App Service

Your code "fixes" the issues with point 4 for root domains, but how do you manage point 2? Since it is not possible to create in Azure DNS a CNAME for a root domain, in theory the process stops at point 2 so having the code handling point 4 is not useful.

Am I missing something?

RichMercer commented 4 years ago

I thought for step 2 it actually created a TXT record, not a CNAME?

n3wt0n commented 4 years ago

I thought for step 2 it actually created a TXT record, not a CNAME?

You are right, sorry my fault. I was referring to an early implementation that is not there anymore. That's the problem on working on too many things on early morning 😁

RichMercer commented 4 years ago

Hi, how do you feel about accepting this PR? Have you had a chance to test it?

n3wt0n commented 4 years ago

Hi, how do you feel about accepting this PR? Have you had a chance to test it?

Didn't have much time to test unfortunately, but I'm reaching out to the Fluent SDK owners to understand what that piece of code actually does...

I think we are probably "missing something" :)

n3wt0n commented 4 years ago

Alright, first of all thanks for your patience.

I've been able to talk with one of the owners of the Fluent SDK so now I have a much clearer picture.

It works this way:

If you already have a hostname bound to your AppService and you just need to associate the SSL cert, then you can remove that part
If instead you need to both create the binding and then associate the cert, you need either the “WithThirdPartyHostnameBinding” or the “WithManagedHostnameBinding”

So now it is down to how we want to implement this. I'd say that, since this is a piece of software that aims to simplify the SSL cert assignment/creation/renewal and nothing more, it would be sufficient letting people know that they need to have a hostname already bound to their AppService. If you agree, then I will accept the PR and add a comment in the README to point that out.

RichMercer commented 4 years ago

I agree that this is about SSL creation, so I think creating hostname bindings is outside of that scope.