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

Enhancement: Support Preflight Validation #501

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ms-zhenhua
Copy link
Collaborator

@ms-zhenhua ms-zhenhua commented May 10, 2024

Related Document:

https://armwiki.azurewebsites.net/api_contracts/preflight.html

Test Results

--- PASS: TestAccGenericResource_defaultParentId (169.21s)
--- PASS: TestAccGenericResource_disablePreflightValidation (173.48s)
--- PASS: TestAccGenericResource_preflightResourceGroupLevelValidation (186.22s)
--- PASS: TestAccGenericResource_ignoreChanges (213.73s)
--- PASS: TestAccGenericResource_basic (255.66s)
--- PASS: TestAccGenericResource_ignoreChangesArray (265.48s)
--- PASS: TestAccGenericResource_completeBody (271.28s)
--- PASS: TestAccGenericResource_defaultsNaming (341.81s)
--- PASS: TestAccGenericResource_requiresImport (150.80s)
--- PASS: TestAccGenericResource_defaultLocation (379.96s)
--- PASS: TestAccGenericResource_defaultsNotApplicable (213.14s)
--- PASS: TestAccGenericResource_complete (214.40s)
--- PASS: TestAccGenericResource_importWithApiVersion (228.25s)
--- PASS: TestAccGenericResource_locks (207.35s)
--- PASS: TestAccGenericResource_subscriptionScope (124.44s)
--- PASS: TestAccGenericResource_nullLocation (611.74s)
--- PASS: TestAccGenericResource_ignoreCasing (345.26s)
--- PASS: TestAccGenericResource_identity (459.51s)
--- PASS: TestAccGenericResource_defaultTags (507.41s)
--- PASS: TestAccGenericResource_invalidVersionUpdate (318.07s)
--- PASS: TestAccGenericResource_dynamicSchema (214.23s)
--- PASS: TestAccGenericResource_deleteLROEndsWithNotFoundError (344.94s)
--- PASS: TestAccGenericResource_secretsInAsterisks (748.64s)
--- PASS: TestAccGenericResource_ignoreMissingProperty (420.70s)
PASS
ok github.com/Azure/terraform-provider-azapi/internal/services 842.574s

@ms-zhenhua ms-zhenhua marked this pull request as ready for review May 10, 2024 06:28
@ms-zhenhua ms-zhenhua requested a review from ms-henglu May 10, 2024 06:29
Copy link
Collaborator

@ms-henglu ms-henglu left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! It mostly LGTM, just some minor suggestions.

requestBody["scope"] = requestPlan.ParentID.ValueString()
requestBody["resources"] = []map[string]interface{}{resourceBody}

_, err = client.Action(ctx, "/providers/Microsoft.Resources", "validateResources", "2020-10-01", "POST", requestBody)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please define the request body struct for validateResources API instead of using the generic struct.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated. Thanks.

@@ -929,6 +938,40 @@ func expandBody(body map[string]interface{}, model AzapiResourceModel) diag.Diag
return diag.Diagnostics{}
}

func preflightValidation(ctx context.Context, client *clients.ResourceClient, requestPlan *AzapiResourceModel, body map[string]interface{}) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use id parse.ResourceId instead of requestPlan *AzapiResourceModel, because the necessary information could be retrieved by the parse.ResourceId.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated. Thanks.

@@ -502,6 +502,15 @@ func (r *AzapiResource) CreateUpdate(ctx context.Context, requestPlan tfsdk.Plan
defer locks.UnlockByID(lockId)
}

// preflight validation: currently only supports the subscription scope validation
if r.ProviderData.Features.EnablePreflight && isNewResource && strings.Contains(strings.ToLower(plan.ParentID.ValueString()), "/subscriptions/") {
Copy link
Member

Choose a reason for hiding this comment

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

Could we use strings.HasPrefix() here as the /subscriptions/ must always be at the start?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated. Thanks.

@ms-henglu ms-henglu added this to the v1.14.0 milestone May 14, 2024
@ms-zhenhua ms-zhenhua requested a review from ms-henglu May 15, 2024 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants