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

FrontDoorTlsProtocolType missing after 2019-04-01 #6882

Closed
timja opened this issue Jan 14, 2020 · 9 comments
Closed

FrontDoorTlsProtocolType missing after 2019-04-01 #6882

timja opened this issue Jan 14, 2020 · 9 comments
Assignees
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. CodeGen Issues that relate to code generation 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-author-feedback Workflow: More information is needed from author to address the issue. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Network - Front Door Service: Azure Front Door no-recent-activity There has been no recent activity on this issue.

Comments

@timja
Copy link

timja commented Jan 14, 2020

Bug Report

  • import path of package in question, e.g. .../services/frontdoor/mgmt/2019-11-01/frontdoor
  • 38.1.0
  • output of go version
    go version go1.13 darwin/amd64

Hi 👋

I'm trying to upgrade terraform to the newest version of the SDK, it seems that FrontDoorTlsProtocolType is missing after 2019-04-01.
But it still appears to be in the rest api spec?

Something wrong in the generation? https://github.com/Azure/azure-rest-api-specs/blob/master/specification/frontdoor/resource-manager/Microsoft.Network/stable/2019-05-01/frontdoor.json#L1395-L1405

(I can't see a frontdoor.json after 2019-05-01 so I assume that means the API spec hasn't changed since then, that's also the version that the constant disappeared in).

https://github.com/Azure/azure-sdk-for-go/blob/master/services/frontdoor/mgmt/2019-04-01/frontdoor/models.go#L775
https://github.com/Azure/azure-sdk-for-go/blob/master/services/frontdoor/mgmt/2019-11-01/frontdoor/models.go#L974

  • What happened?
    Missing field FrontDoorTlsProtocolType

  • What did you expect or want to happen?
    The field to be there

  • How can we reproduce it?
    Look at above links

  • Anything we should know about your environment.

hashicorp/terraform-provider-azurerm#5385

@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. Network labels Jan 15, 2020
@ArcturusZhang
Copy link
Member

Hi @timja thanks for the issue.
You are correct, the spec for frontdoor is the same as 2019-05-01, you can find the corresponding spec definition in the readme file
As for the FrontDoorTlsProtocolType type, as you see in the sdk repo, the model.go file has not been changed since v34.0.0. I will have some investigation on why that happens. In the mean time, that field is an enum, and it could only have one possible value: ServerNameIndication, you could just hard code that value as a workaround for now.

@ArcturusZhang
Copy link
Member

Hi @jhendrixMSFT could you please have an explanation from the code gen side why it differs from 2019-04-01 and 2019-05-01 (and higher)?

@timja
Copy link
Author

timja commented Jan 15, 2020

(As you can see in the linked terraform PR I’ve hard coded it with a link to this issue)

@jhendrixMSFT
Copy link
Member

This enum has one value and is required. In this case the codegen is supposed to treat it as a constant, i.e. you shouldn't be able to even set the field, the value is sent without user intervention. This works for constant parameter values but is apparently broken for model properties. Will work on a fix.

@jhendrixMSFT jhendrixMSFT added the CodeGen Issues that relate to code generation label Jan 15, 2020
@jhendrixMSFT jhendrixMSFT self-assigned this Jan 15, 2020
@timMSFT timMSFT added Network - Front Door Service: Azure Front Door and removed Network labels Aug 13, 2020
@RickWinter RickWinter assigned chamons and unassigned jhendrixMSFT Jul 19, 2021
@RickWinter RickWinter added this to the [2022] January milestone Jul 19, 2021
@chamons
Copy link
Contributor

chamons commented Jul 22, 2021

I just tested latest autorest.go and this is what it generated:

// FrontDoorTLSProtocolType - Defines the TLS extension protocol that is used for secure delivery
type FrontDoorTLSProtocolType string

...
type CustomHTTPSConfiguration struct {
	// REQUIRED; Defines the source of the SSL certificate
	CertificateSource *FrontDoorCertificateSource `json:"certificateSource,omitempty"`

	// REQUIRED; The minimum TLS version required from the clients to establish an SSL handshake with Front Door.
	MinimumTLSVersion *MinimumTLSVersion `json:"minimumTlsVersion,omitempty"`

	// REQUIRED; Defines the TLS extension protocol that is used for secure delivery
	ProtocolType *FrontDoorTLSProtocolType `json:"protocolType,omitempty"`

	// Parameters required for enabling SSL with Front Door-managed certificates (if certificateSource=FrontDoor)
	FrontDoorCertificateSourceParameters *FrontDoorCertificateSourceParameters `json:"frontDoorCertificateSourceParameters,omitempty"`

	// KeyVault certificate source parameters (if certificateSource=AzureKeyVault)
	KeyVaultCertificateSourceParameters *KeyVaultCertificateSourceParameters `json:"keyVaultCertificateSourceParameters,omitempty"`
}

I am uncertain if that means this is fixed, or broken worse given your comment on constants @jhendrixMSFT

@lirenhe lirenhe assigned jhendrixMSFT and unassigned chamons Sep 10, 2021
@lirenhe
Copy link
Member

lirenhe commented Dec 24, 2021

@timja, could you help to confirm whether this problem is fixed in the latest version of the SDK?
https://github.com/Azure/azure-sdk-for-go/tree/main/services/frontdoor/mgmt/2020-11-01/frontdoor

@lirenhe lirenhe added the needs-author-feedback Workflow: More information is needed from author to address the issue. label Dec 24, 2021
@timja
Copy link
Author

timja commented Dec 24, 2021

From @chamons comment it's not fixed.

Nothing appears to have changed:

ProtocolType *string `json:"protocolType,omitempty"`

According to @jhendrixMSFT this should just be a constant and not be settable.

(note I haven't tried to upgrade and I raised this nearly 2 years ago so just doing my best from checking the code)

@ghost ghost added needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed needs-author-feedback Workflow: More information is needed from author to address the issue. labels Dec 24, 2021
@RickWinter RickWinter added the bug This issue requires a change to an existing behavior in the product in order to be resolved. label Dec 29, 2021
@jhendrixMSFT
Copy link
Member

Unfortunately, I misspoke earlier. When the enum is marked as modelAsString: true, it means that this enum should be treated as extensible, i.e. more values might be added in the future. In this case, it's not treated as a constant, so callers are expected to provide the value, even if there's only one to choose from.

@lirenhe lirenhe added the needs-author-feedback Workflow: More information is needed from author to address the issue. label Jan 12, 2022
@ghost ghost added the no-recent-activity There has been no recent activity on this issue. label Jan 19, 2022
@ghost
Copy link

ghost commented Jan 19, 2022

Hi, we're sending this friendly reminder because we haven't heard back from you in a while. We need more information about this issue to help address it. Please be sure to give us your input within the next 7 days. If we don't hear back from you within 14 days of this comment the issue will be automatically closed. Thank you!

@ghost ghost closed this as completed Feb 2, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2023
This issue was closed.
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. CodeGen Issues that relate to code generation 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-author-feedback Workflow: More information is needed from author to address the issue. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Network - Front Door Service: Azure Front Door no-recent-activity There has been no recent activity on this issue.
Projects
None yet
Development

No branches or pull requests

8 participants