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

New Resource: azurerm_certificate_order_key_vault_store #25464

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion .github/labeler-issue-triage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ service/app-configuration:
- '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_app_configuration((.|\n)*)###'

service/app-service:
- '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(app_service_environment_v3\W+|app_service_environment_v3\W+|app_service_source_control\W+|app_service_source_control_slot\W+|function_app_active_slot\W+|function_app_function\W+|function_app_hybrid_connection\W+|linux_function_app\W+|linux_function_app\W+|linux_function_app_slot\W+|linux_web_app\W+|linux_web_app\W+|linux_web_app_slot\W+|service_plan|source_control_token|static_web_app|web_app_|windows_function_app\W+|windows_function_app\W+|windows_function_app_slot\W+|windows_web_app\W+|windows_web_app\W+|windows_web_app_slot\W+)((.|\n)*)###'
- '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(app_service_certificate_order_certificate\W+|app_service_environment_v3\W+|app_service_environment_v3\W+|app_service_source_control\W+|app_service_source_control_slot\W+|function_app_active_slot\W+|function_app_function\W+|function_app_hybrid_connection\W+|linux_function_app\W+|linux_function_app\W+|linux_function_app_slot\W+|linux_web_app\W+|linux_web_app\W+|linux_web_app_slot\W+|service_plan|source_control_token|static_web_app|web_app_|windows_function_app\W+|windows_function_app\W+|windows_function_app_slot\W+|windows_web_app\W+|windows_web_app\W+|windows_web_app_slot\W+)((.|\n)*)###'

service/application-insights:
- '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_application_insights((.|\n)*)###'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,243 @@
package appservice

import (
"context"
"fmt"
"time"

"github.com/hashicorp/go-azure-helpers/lang/pointer"
"github.com/hashicorp/go-azure-helpers/lang/response"
"github.com/hashicorp/go-azure-helpers/resourcemanager/commonids"
"github.com/hashicorp/go-azure-helpers/resourcemanager/commonschema"
"github.com/hashicorp/go-azure-sdk/resource-manager/web/2023-01-01/appservicecertificateorders"
"github.com/hashicorp/terraform-provider-azurerm/internal/sdk"
keyVaultValidate "github.com/hashicorp/terraform-provider-azurerm/internal/services/keyvault/validate"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/web/validate"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/suppress"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation"
)

type CertificateOrderCertificateResource struct{}

type CertificateOrderCertificateModel struct {
Name string `tfschema:"name"`
CertificateOrderId string `tfschema:"certificate_order_id"`
Location string `tfschema:"location"`
KeyVaultId string `tfschema:"key_vault_id"`
KeyVaultSecretName string `tfschema:"key_vault_secret_name"`
}

func (r CertificateOrderCertificateResource) Arguments() map[string]*pluginsdk.Schema {
return map[string]*pluginsdk.Schema{
"name": {
Type: pluginsdk.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validation.StringIsNotEmpty,
},

"certificate_order_id": {
Type: pluginsdk.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validate.CertificateOrderID,
},

"key_vault_id": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: commonids.ValidateKeyVaultID,
// TODO -- remove when issue https://github.com/Azure/azure-rest-api-specs/issues/28498 is addressed
DiffSuppressFunc: suppress.CaseDifference,
},

"key_vault_secret_name": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: keyVaultValidate.NestedItemName,
},
Copy link
Member

Choose a reason for hiding this comment

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

Could these be replaced by key_vault_secret_id?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm afraid not, because we are not referring to a key vault secret, instead, we are creating the secret in the key vault with a name specified.

}
}

func (r CertificateOrderCertificateResource) Attributes() map[string]*pluginsdk.Schema {
return map[string]*pluginsdk.Schema{
"location": commonschema.LocationComputed(),

"type": {
Type: pluginsdk.TypeString,
Computed: true,
},
}
}

func (r CertificateOrderCertificateResource) ModelObject() interface{} {
return &CertificateOrderCertificateModel{}
}

func (r CertificateOrderCertificateResource) ResourceType() string {
return "azurerm_app_service_certificate_order_certificate"
Copy link
Member

Choose a reason for hiding this comment

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

Can we rename this so it's a bit clearer at first glance what this does

Suggested change
return "azurerm_app_service_certificate_order_certificate"
return "azurerm_app_service_certificate_order_key_vault_store"

}

func (r CertificateOrderCertificateResource) Create() sdk.ResourceFunc {
return sdk.ResourceFunc{
Timeout: 60 * time.Minute,
Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error {
var certificateOrderCertificate CertificateOrderCertificateModel
if err := metadata.Decode(&certificateOrderCertificate); err != nil {
return err
}

client := metadata.Client.AppService.AppServiceCertificatesOrderClient
subscriptionId := metadata.Client.Account.SubscriptionId

certificateOrderId, err := appservicecertificateorders.ParseCertificateOrderID(certificateOrderCertificate.CertificateOrderId)
if err != nil {
return fmt.Errorf("parsing certificate order error %+v", err)
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned previously, it isn't necessary to wrap the error messages coming from the parsers

Suggested change
return fmt.Errorf("parsing certificate order error %+v", err)
return err

}
id := appservicecertificateorders.NewCertificateID(subscriptionId, certificateOrderId.ResourceGroupName, certificateOrderId.CertificateOrderName, certificateOrderCertificate.Name)

keyVaultId, err := commonids.ParseKeyVaultIDInsensitively(certificateOrderCertificate.KeyVaultId)
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't parse IDs that are input by the user insensitively, this would allow casing differences in the ID to creep into their state.

Suggested change
keyVaultId, err := commonids.ParseKeyVaultIDInsensitively(certificateOrderCertificate.KeyVaultId)
keyVaultId, err := commonids.ParseKeyVaultID(certificateOrderCertificate.KeyVaultId)

if err != nil {
return err
}
kvId := commonids.NewKeyVaultID(keyVaultId.SubscriptionId, keyVaultId.ResourceGroupName, keyVaultId.VaultName)
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't need to be reconstructed

Suggested change
kvId := commonids.NewKeyVaultID(keyVaultId.SubscriptionId, keyVaultId.ResourceGroupName, keyVaultId.VaultName)


existing, err := client.GetCertificate(ctx, id)
if err != nil && !response.WasNotFound(existing.HttpResponse) {
return fmt.Errorf("retreiving %s: %v", id, err)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("retreiving %s: %v", id, err)
return fmt.Errorf("retrieving %s: %v", id, err)

}
if !response.WasNotFound(existing.HttpResponse) {
return metadata.ResourceRequiresImport(r.ResourceType(), id)
}

certOrderCertificate := appservicecertificateorders.AppServiceCertificateResource{
Name: pointer.To(certificateOrderCertificate.Name),
Properties: &appservicecertificateorders.AppServiceCertificate{
KeyVaultId: pointer.To(kvId.ID()),
KeyVaultSecretName: pointer.To(certificateOrderCertificate.KeyVaultSecretName),
},
}

if err := client.CreateOrUpdateCertificateThenPoll(ctx, id, certOrderCertificate); err != nil {
return fmt.Errorf("creating %s: %+v", id, err)
}

metadata.SetID(id)

return nil
},
}
}

func (r CertificateOrderCertificateResource) Read() sdk.ResourceFunc {
return sdk.ResourceFunc{
Timeout: 5 * time.Minute,
Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error {
client := metadata.Client.AppService.AppServiceCertificatesOrderClient
id, err := appservicecertificateorders.ParseCertificateIDInsensitively(metadata.ResourceData.Id())
Copy link
Member

Choose a reason for hiding this comment

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

This ID should have been created with the correct casing in the create so we shouldn't be parsing this insensitively

Suggested change
id, err := appservicecertificateorders.ParseCertificateIDInsensitively(metadata.ResourceData.Id())
id, err := appservicecertificateorders.ParseCertificateID(metadata.ResourceData.Id())

if err != nil {
return err
}

certificateOrderCertificate, err := client.GetCertificate(ctx, *id)
if err != nil {
if response.WasNotFound(certificateOrderCertificate.HttpResponse) {
return metadata.MarkAsGone(id)
}
return fmt.Errorf("reading %s: %+v", id, err)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("reading %s: %+v", id, err)
return fmt.Errorf("retrieving %s: %+v", id, err)

}

state := CertificateOrderCertificateModel{
Name: id.CertificateName,
}

certificateOrderId := appservicecertificateorders.NewCertificateOrderID(id.SubscriptionId, id.ResourceGroupName, id.CertificateOrderName)
state.CertificateOrderId = certificateOrderId.ID()

if model := certificateOrderCertificate.Model; model != nil {
state.Location = model.Location
Copy link
Member

Choose a reason for hiding this comment

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

Location should be normalized

Suggested change
state.Location = model.Location
state.Location = location.Normalize(model.Location)

if props := model.Properties; props != nil {
if props.KeyVaultId != nil {
keyVaultId, err := commonids.ParseKeyVaultIDInsensitively(*props.KeyVaultId)
Copy link
Member

Choose a reason for hiding this comment

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

Is the API returning the key vault ID with incorrect casing? If it isn't then we should be parsing this sensitively

Suggested change
keyVaultId, err := commonids.ParseKeyVaultIDInsensitively(*props.KeyVaultId)
keyVaultId, err := commonids.ParseKeyVaultID(*props.KeyVaultId)

if err != nil {
return err
}
state.KeyVaultId = keyVaultId.ID()
}
state.KeyVaultSecretName = pointer.From(props.KeyVaultSecretName)
}
}
if err := metadata.Encode(&state); err != nil {
return fmt.Errorf("encoding: %+v", err)
}

return nil
},
}
}

func (r CertificateOrderCertificateResource) Delete() sdk.ResourceFunc {
return sdk.ResourceFunc{
Timeout: 60 * time.Minute,
Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error {
id, err := appservicecertificateorders.ParseCertificateIDInsensitively(metadata.ResourceData.Id())
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
id, err := appservicecertificateorders.ParseCertificateIDInsensitively(metadata.ResourceData.Id())
id, err := appservicecertificateorders.ParseCertificateID(metadata.ResourceData.Id())

if err != nil {
return err
}

client := metadata.Client.AppService.AppServiceCertificatesOrderClient
metadata.Logger.Infof("deleting %s", id)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think logging delete messages is a convention that we follow in the provider

Suggested change
metadata.Logger.Infof("deleting %s", id)


if _, err := client.DeleteCertificate(ctx, *id); err != nil {
return fmt.Errorf("deleting %s: %+v", id, err)
}

return nil
},
}
}

func (r CertificateOrderCertificateResource) Update() sdk.ResourceFunc {
return sdk.ResourceFunc{
Timeout: 60 * time.Minute,
Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error {
id, err := appservicecertificateorders.ParseCertificateIDInsensitively(metadata.ResourceData.Id())
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
id, err := appservicecertificateorders.ParseCertificateIDInsensitively(metadata.ResourceData.Id())
id, err := appservicecertificateorders.ParseCertificateID(metadata.ResourceData.Id())

if err != nil {
return err
}

client := metadata.Client.AppService.AppServiceCertificatesOrderClient

var state CertificateOrderCertificateModel
if err := metadata.Decode(&state); err != nil {
return fmt.Errorf("decoding: %+v", err)
}

existing, err := client.GetCertificate(ctx, *id)
if err != nil {
return fmt.Errorf("reading %s: %+v", id, err)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("reading %s: %+v", id, err)
return fmt.Errorf("retrieving %s: %+v", id, err)

}

model := *existing.Model

if metadata.ResourceData.HasChange("key_vault_id") {
model.Properties.KeyVaultId = pointer.To(state.KeyVaultId)
}

if metadata.ResourceData.HasChange("key_vault_secret_name") {
model.Properties.KeyVaultSecretName = pointer.To(state.KeyVaultSecretName)
}

if err := client.CreateOrUpdateCertificateThenPoll(ctx, *id, model); err != nil {
return fmt.Errorf("updating %s: %+v", id, err)
}

return nil
},
}
}

func (r CertificateOrderCertificateResource) IDValidationFunc() pluginsdk.SchemaValidateFunc {
return appservicecertificateorders.ValidateCertificateID
}
Loading
Loading