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

web/CertificatesClient should not force a value for the "password" property #6498

Closed
tiwood opened this issue Dec 6, 2019 · 16 comments
Closed
Assignees
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. customer-reported Issues that are reported by GitHub users external to the Azure organization. Mgmt This issue is related to a management-plane library. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Service Attention Workflow: This issue is responsible by Azure service team. Web Apps

Comments

@tiwood
Copy link

tiwood commented Dec 6, 2019

Bug Report

  • import path of package
    "github.com/Azure/azure-sdk-for-go/services/web/mgmt/2019-08-01/web"
  • SDK version master
  • go version go1.13.4 darwin/amd64

You can now create App Service Managed Certificates by providing a Certificate struct with the following properties to the CertificatesClient.CreateOrUpdate function:

certificate := web.Certificate{
		CertificateProperties: &web.CertificateProperties{
			CanonicalName: utils.String(name),
			ServerFarmID:  utils.String(appServicePlanID),
		},
		Location: utils.String(location),
		Tags:     tags.Expand(t),
	}

Note that Password is not required and should not be set. If a value for Password is set, the API will return GatewayTimeout errors.

Unfortunately, the SDK currently validates that Password is not nil. This has to be fixed to enable the creation of App Service Managed Certificates.

The validation happens here:

Chain: []validation.Constraint{{Target: "certificateEnvelope.CertificateProperties.Password", Name: validation.Null, Rule: true, Chain: nil}}}}}}); err != nil {

@ArcturusZhang ArcturusZhang added customer-reported Issues that are reported by GitHub users external to the Azure organization. Mgmt This issue is related to a management-plane library. Service Attention Workflow: This issue is responsible by Azure service team. Web Apps labels Dec 6, 2019
@triage-new-issues triage-new-issues bot removed the triage label Dec 6, 2019
@ghost
Copy link

ghost commented Dec 6, 2019

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @AzureAppServiceCLI @antcp

1 similar comment
@ghost
Copy link

ghost commented Dec 6, 2019

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @AzureAppServiceCLI @antcp

@ArcturusZhang
Copy link
Member

Hi @tiwood thanks for opening this issue!
I just checked the corresponding swagger for this service. It turns out that the password field is marked as required in the swagger.
I have tagged this issue, so the corresponding service team would have a look on this issue.

@tiwood
Copy link
Author

tiwood commented Dec 17, 2019

Hi @ArcturusZhang, can you provide an update?

@ArcturusZhang
Copy link
Member

Hi @ArcturusZhang, can you provide an update?

I am afraid that I cannot provide a fix directly in go SDK. This is written in the swagger which is managed by the service team.
Would you please open an issue in the azure-rest-api-specs repo about this and referring this issue then?

@tombuildsstuff
Copy link
Contributor

@ArcturusZhang is there a timeframe for this Swagger getting fixed?

@drdamour
Copy link

Is this the upstream issue Azure/azure-rest-api-specs#5029 ?

@ArcturusZhang
Copy link
Member

Is this the upstream issue Azure/azure-rest-api-specs#5029 ?

Indeed, thanks for reminding me this. I have tagged the upstream issue to inform the corresponding service team.

@ArcturusZhang
Copy link
Member

@ArcturusZhang is there a timeframe for this Swagger getting fixed?

Sorry that I do not have a timeframe for this issue...
I will try to make some internal email directly to the corresponding service team

@panchagnula
Copy link

@ArcturusZhang AFAIK password for a cert is always a required property if you are uploading a certificate - for a managed certificate we create the certificate & how are you trying to use this for managed certs? Can you refer to the CLI command here & check how are using the API terraform scripts to use this? https://docs.microsoft.com/en-us/cli/azure/webapp/config/ssl?view=azure-cli-latest#az-webapp-config-ssl-create

@AdamCoulterOz
Copy link

AdamCoulterOz commented Aug 5, 2020

@panchagnula

@ArcturusZhang AFAIK password for a cert is always a required property if you are uploading a certificate - for a managed certificate we create the certificate & how are you trying to use this for managed certs? Can you refer to the CLI command here & check how are using the API terraform scripts to use this? https://docs.microsoft.com/en-us/cli/azure/webapp/config/ssl?view=azure-cli-latest#az-webapp-config-ssl-create

I'm pretty sure its something like this .... but there is a problem with the az cli bind if the webapp and service plans are in different resource groups (tracked by Azure/azure-cli#13929 which @panchagnula just commented there is no ETA to fix)

Step 1: Create Managed Certificate... which works perfectly

az webapp config ssl create --resource-group {} --name {webapp} --hostname {FQDN}

Step 2: get the {certificate_thumbprint} from the json object output from 1

Step 3: Bind the certificate to the website custom domain name

az webapp config ssl bind --certificate-thumbprint {certificate_thumbprint} --name {webapp} --resource-group {} --ssl-type SNI

@AdamCoulterOz
Copy link

AdamCoulterOz commented Aug 5, 2020

as for the managed cert creation, the az cli does this (using Azure-SDK-For-Python), and I've tested it working:

PUT /subscriptions/{sub}/resourceGroups/{rg}/providers/Microsoft.Web/certificates/{custom-hostname}?api-version=2019-08-01

Body:

{
    "location": "Australia East",
    "properties": {
        "password": "",
        "serverFarmId": "/subscriptions/{sub}/resourceGroups/{rg}/providers/Microsoft.Web/serverfarms/{serviceplan}",
        "canonicalName": "{custom-hostname}"
    }
}

@tiwood have you tried setting the password to empty string? ""

@martinjt
Copy link

Is anyone actively pushing this? it's blocking some other things by the look of it.

@martinjt
Copy link

martinjt commented Oct 3, 2020

@tombuildsstuff Don't suppose you have someone at Azure you could nudge to sort the dependent issue?

@panchagnula
Copy link

@martinjt on the API end app service doesn't enforce the password option. A passoword is required when a cert (local) is being uploaded to a webapp. If its a managed certificate or certificate from a AKV - then you don't need the password info - the API has it optional. I am not aware of the GO SDK but I can ensure the API doesn't enforce - so please check the usage of the API. Thanks!

@AdamCoulterOz
Copy link

So I've just tested this out, and it looks like it works if you set the password to an empty string like this:

certificate := web.Certificate{
	CertificateProperties: &web.CertificateProperties{
		CanonicalName: utils.String(name),
		ServerFarmID:  utils.String(appServicePlanID),
		Password:      new(string),
	},
	Location: utils.String(location),
	Tags:     tags.Expand(t),
}

This was my suspicion about three months ago when I posted earlier, but haven't had a chance till now to test my theory. I have created a PR to fix this for the new resource here:

hashicorp/terraform-provider-azurerm#9378

@RickWinter RickWinter added the bug This issue requires a change to an existing behavior in the product in order to be resolved. label Jul 19, 2021
@ghost ghost added the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Jul 19, 2021
@RickWinter RickWinter added this to the Backlog milestone Jul 19, 2021
@tiwood tiwood closed this as completed Nov 12, 2023
@RickWinter RickWinter removed this from the Backlog milestone Dec 6, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Mar 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. customer-reported Issues that are reported by GitHub users external to the Azure organization. Mgmt This issue is related to a management-plane library. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Service Attention Workflow: This issue is responsible by Azure service team. Web Apps
Projects
None yet
Development

No branches or pull requests

8 participants