New Resource: `azurerm_qumulo_file_system` by teowa · Pull Request #28704 · hashicorp/terraform-provider-azurerm

teowa · 2025-02-06T09:24:19Z

Community Note

Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for PR followers and do not help prioritize for review

Description

supersede #22911 , which had been reviewed but was blocked by upstream API
New Resource: azurerm_qumulo_file_system

PR Checklist

I have followed the guidelines in our Contributing Documentation.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.
I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
For example: “resource_name_here - description of change e.g. adding property new_property_name_here”

Changes to existing Resource / Data Source

I have added an explanation of what my changes do and why I'd like you to include them (This may be covered by linking to an issue above, but may benefit from additional explanation).
I have written new tests for my resource or datasource changes & updated any relevent documentation.
I have successfully run tests with my changes locally. If not, please provide details on testing challenges that prevented you running the tests.
(For changes that include a state migration only). I have manually tested the migration path between relevant versions of the provider.

Testing

My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

NOTE: service team asks to run the acctest in canary region, for example, "eastasia", "centralus2euap"

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

New Resource: azurerm_qumulo_file_system [GH-00000]

This is a (please select all that apply):

Bug Fix
New Feature (ie adding a service, resource, or data source)
Enhancement
Breaking Change

Related Issue(s)

Fixes #0000

Note

If this PR changes meaningfully during the course of review please update the title and description as required.

…m into qumulo_file_system

teowa · 2025-02-17T02:25:24Z

ms-zhenhua

Hi @teowa ,

Thanks for this PR - I've taken a look through and left some comments inline. If we can fix those up, this should be good to go 👍

ms-zhenhua · 2025-03-21T06:40:32Z

internal/services/qumulo/qumulo_file_system_resource.go

+var (
+	_ sdk.Resource           = FileSystemResource{}
+	_ sdk.ResourceWithUpdate = FileSystemResource{}
+)


Suggested change

var (

_ sdk.Resource = FileSystemResource{}

_ sdk.ResourceWithUpdate = FileSystemResource{}

)

var _ sdk.ResourceWithUpdate = FileSystemResource{}

internal/services/qumulo/qumulo_file_system_resource.go

ms-zhenhua · 2025-03-24T03:27:24Z

website/docs/r/qumulo_file_system.html.markdown

+  admin_password      = ")^X#ZX#JRyIY}t9"
+  availability_zone   = "1"
+  delegated_subnet_id = azurerm_subnet.example.id
+  marketplace_plan_id = "qumulo-on-azure-v1%%gmz7xq9ge3py%%P1M"


why is marketplace_plan_id set such a strange value? Is there any documents guiding users to set this property?

fixed, the plan_id value is not limited to azure-native-qumulo-v3. Contact Azure Qumulo support for other possible values.

ms-zhenhua · 2025-03-24T07:09:29Z

internal/services/qumulo/qumulo_file_system_resource.go

+			ValidateFunc: commonids.ValidateSubnetID,
+		},
+
+		"user_email_address": {


Suggested change

"user_email_address": {

"email": {

ms-zhenhua · 2025-03-24T07:10:23Z

internal/services/qumulo/qumulo_file_system_resource.go

+			Sensitive: true,
+		},
+
+		"marketplace_plan_id": {


Suggested change

"marketplace_plan_id": {

"plan_id": {

ms-zhenhua · 2025-03-24T07:47:53Z

internal/services/qumulo/qumulo_file_system_resource_test.go

+
+	resp, err := clients.Qumulo.FileSystemsClient.Get(ctx, *id)
+	if err != nil {
+		return nil, fmt.Errorf("reading %s: %+v", *id, err)


Suggested change

return nil, fmt.Errorf("reading %s: %+v", *id, err)

return nil, fmt.Errorf("retrieving %s: %+v", *id, err)

ms-zhenhua · 2025-03-24T07:48:14Z

internal/services/qumulo/qumulo_file_system_resource_test.go

+		return nil, fmt.Errorf("reading %s: %+v", *id, err)
+	}
+
+	return utils.Bool(resp.Model != nil), nil


Suggested change

return utils.Bool(resp.Model != nil), nil

return pointer.To(resp.Model != nil), nil

ms-zhenhua · 2025-03-24T07:48:53Z

internal/services/qumulo/validate/file_system_name.go

+func FileSystemName(v interface{}, k string) (warnings []string, errors []error) {
+	value := v.(string)
+	if matched := regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9-]{0,13}[a-zA-Z0-9]$`).Match([]byte(value)); !matched {
+		errors = append(errors, fmt.Errorf("%q must be between 2 and 15 characters in length, must not begin or end with a hyphen and may only contain alphanumeric characters and hyphens", k))


Suggested change

errors = append(errors, fmt.Errorf("%q must be between 2 and 15 characters in length, must not begin or end with a hyphen and may only contain alphanumeric characters and hyphens", k))

errors = append(errors, fmt.Errorf("`%q` must be between 2 and 15 characters in length, must not begin or end with a hyphen and may only contain alphanumeric characters and hyphens", k))

ms-zhenhua · 2025-03-24T07:49:27Z

website/docs/r/qumulo_file_system.html.markdown

+layout: "azurerm"
+page_title: "Azure Resource Manager: azurerm_qumulo_file_system"
+description: |-
+  Manages a Azure Native Qumulo Scalable File System.


Suggested change

Manages a Azure Native Qumulo Scalable File System.

Manages an Azure Native Qumulo Scalable File System.

ms-zhenhua · 2025-03-24T07:49:37Z

website/docs/r/qumulo_file_system.html.markdown

+
+# azurerm_qumulo_file_system
+
+Manages a Azure Native Qumulo Scalable File System.


Suggested change

Manages a Azure Native Qumulo Scalable File System.

Manages an Azure Native Qumulo Scalable File System.

…der-azurerm into qumulo_file_system

…m into qumulo_file_system

teowa · 2025-04-18T08:20:57Z

Hi @stephybun , thanks for reviewing this. Please kindly take another look.

katbyte · 2025-04-21T18:26:55Z

internal/services/qumulo/qumulo_file_system_resource.go

+		"offer_id": {
+			Type:     pluginsdk.TypeString,
+			Required: true,
+			ForceNew: true,
+			// the value can be "qumulo-saas-mpp"
+			ValidateFunc: validation.StringIsNotEmpty,
+		},
+
+		"plan_id": {
+			Type:     pluginsdk.TypeString,
+			Required: true,
+			ForceNew: true,
+			// the value includes but is not limitted to ["azure-native-qumulo-v3"]
+			ValidateFunc: validation.StringIsNotEmpty,
+		},


we should use the hardcoded values you had before as defaults here?

According to the swagger publisher_id shouldn't be a required property but is marked as such in the schema here?

added default to the publisher_id

default also added to offer_id and plan_id.

teowa · 2025-04-23T09:34:27Z

Hi @katbyte , thanks for reviewing this! I have added inline reply, please kindly take another look.

stephybun · 2025-04-23T09:45:20Z

internal/services/qumulo/qumulo_file_system_resource.go

+			Type:     pluginsdk.TypeString,
+			Required: true,
+			ForceNew: true,
+			// the value includes but is not limitted to ["azure-native-qumulo-v3"]


Suggested change

// the value includes but is not limitted to ["azure-native-qumulo-v3"]

// the value includes but is not limited to ["azure-native-qumulo-v3"]

…m into qumulo_file_system

teowa · 2025-05-09T11:57:16Z

Hi @stephybun @katbyte , thanks for reviewing! I have updated the code as in the review comment, could you please take another look? Thanks!

katbyte

Thanks @teowa - this looks good except for one minor comment about validation. once thats address & passing tests shared this is g2g

katbyte · 2025-05-26T19:53:37Z

internal/services/qumulo/qumulo_file_system_resource.go

+			Type:         pluginsdk.TypeString,
+			Required:     true,
+			ForceNew:     true,
+			ValidateFunc: validation.StringIsNotEmpty,


could we validate the email address, should be able to use the net/mail package mail.ParseAddress(email)

teowa · 2025-05-27T08:16:39Z

Hi @katbyte , I am unable to test this online because the test subscription does not allow for Azure Marketplace payment.

testcase.go:173: Step 1/3 error: Error running apply: exit status 1
        Error: creating File System (Subscription: "*******"
        Resource Group Name: "acctestrg-qumulo-9wm0p"
        File System Name: "acctest-9wm0p"): performing CreateOrUpdate: unexpected status 400 (400 Bad Request) with error: ResourceCreationValidateFailed: MarketplaceValidation, MarketplaceValidation, SaaS Purchase Payment Check Failed as validationResponse was {"isEligible":false,"errorMessage":"Cannot complete purchase as your enrollment for this subscription doesn't allow purchase of marketplace products. Your Azure enrollment administrator can enable purchase of paid marketplace products. for details, see https://go.microsoft.com/fwlink/?linkid=2297238. Correlation Id: '40a4d251-b0fc-4cdb-90fe-242db685cf1e'."}, failed for resoruce id /subscriptions/*******/resourcegroups/acctestrg-qumulo-9wm0p/providers/Qumulo.Storage/fileSystems/acctest-9wm0p, resource name acctest-9wm0p

The test can pass in my local:

katbyte

LGTM 🏖️

stales

github-actions · 2025-07-04T02:26:26Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

teowa added 7 commits August 18, 2023 11:23

qumulo_file_system

18acaf7

terrafmt

a6c7e69

review comment

1460d18

export marketplace_plan_id

71b8bb6

fix review comment

ee2f5f7

Merge branch 'main' of github.com:hashicorp/terraform-provider-azurer…

b99f385

…m into qumulo_file_system

upgrade to 2024-06-19

3e1c5c9

teowa requested a review from a team as a code owner February 6, 2025 09:24

github-actions bot added documentation dependencies size/XL labels Feb 6, 2025

teowa marked this pull request as draft February 6, 2025 09:24

github-actions bot added waiting-response and removed waiting-response labels Feb 6, 2025

fix

2833ac0

teowa force-pushed the qumulo_file_system branch from b43ce3c to 2833ac0 Compare February 6, 2025 10:22

github-actions bot removed the waiting-response label Feb 6, 2025

add canary region for test

0fdb4d1

teowa marked this pull request as ready for review February 13, 2025 09:03

fix test exists check

edb641b

ms-zhenhua suggested changes Mar 24, 2025

View reviewed changes

github-actions bot added the waiting-response label Mar 24, 2025

teowa added 2 commits March 25, 2025 02:49

Merge branch 'qumulo_file_system' of github.com:teowa/terraform-provi…

f590226

…der-azurerm into qumulo_file_system

fix

e601c12

github-actions bot added waiting-response and removed waiting-response labels Mar 25, 2025

Merge branch 'main' of github.com:hashicorp/terraform-provider-azurer…

80cbb4f

…m into qumulo_file_system

github-actions bot removed the waiting-response label Mar 25, 2025

github-actions bot added the waiting-response label Apr 16, 2025

fix

cc25797

github-actions bot removed the waiting-response label Apr 17, 2025

update doc example

b80a4af

katbyte requested changes Apr 21, 2025

View reviewed changes

github-actions bot assigned katbyte Apr 21, 2025

github-actions bot added the waiting-response label Apr 21, 2025

github-actions bot removed the waiting-response label Apr 23, 2025

stephybun reviewed Apr 23, 2025

View reviewed changes

github-actions bot added the waiting-response label Apr 23, 2025

add default to publisher_id

74b7a7f

github-actions bot removed the waiting-response label Apr 24, 2025

teowa added 2 commits April 27, 2025 03:58

Merge branch 'main' of github.com:hashicorp/terraform-provider-azurer…

8cd1a12

…m into qumulo_file_system

add default for offer_id & plan_id

174a094

katbyte requested changes May 26, 2025

View reviewed changes

github-actions bot added the waiting-response label May 26, 2025

add email validate

78437c0

github-actions bot removed the waiting-response label May 27, 2025

katbyte approved these changes Jun 3, 2025

View reviewed changes

katbyte merged commit 6c0417f into hashicorp:main Jun 3, 2025
35 checks passed

katbyte added a commit that referenced this pull request Jun 3, 2025

Update CHANGELOG.md #28704

c358388

github-actions bot added this to the v4.32.0 milestone Jun 3, 2025

wyattfry pushed a commit that referenced this pull request Jun 5, 2025

Update CHANGELOG.md #28704

423399b

github-actions bot locked as resolved and limited conversation to collaborators Jul 4, 2025

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

	return utils.Bool(resp.Model != nil), nil
	return pointer.To(resp.Model != nil), nil

	errors = append(errors, fmt.Errorf("%q must be between 2 and 15 characters in length, must not begin or end with a hyphen and may only contain alphanumeric characters and hyphens", k))
	errors = append(errors, fmt.Errorf("`%q` must be between 2 and 15 characters in length, must not begin or end with a hyphen and may only contain alphanumeric characters and hyphens", k))

	Manages a Azure Native Qumulo Scalable File System.
	Manages an Azure Native Qumulo Scalable File System.


		# azurerm_qumulo_file_system

		Manages a Azure Native Qumulo Scalable File System.

	// the value includes but is not limitted to ["azure-native-qumulo-v3"]
	// the value includes but is not limited to ["azure-native-qumulo-v3"]

Conversation

teowa commented Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Community Note

Description

PR Checklist

Changes to existing Resource / Data Source

Testing

Change Log

Related Issue(s)

Uh oh!

teowa commented Feb 17, 2025

Uh oh!

ms-zhenhua left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teowa commented Apr 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teowa commented Apr 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teowa commented May 9, 2025

Uh oh!

katbyte left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teowa commented Feb 6, 2025 •

edited

Loading