New Resource & DataSource: azurerm_healthcare_service #4221

kutsovmqs · 2019-09-03T08:33:26Z

have been performed using the Azure SDK for Go and the fiunctionality added to the functions in resource_arm_fhir_api_service. DEVOPS-1201

katbyte

Thanks for the pr @kutsovmqs,

I wonder if this resource would be better named azurerm_healthcare_service? the kind property indicates it might support other types then FHIR eventually.

MedalLance · 2019-09-09T23:29:59Z

Hi @katbyte, thank you for your comment and updates, and sorry we didn't put a description of what this PR is. I can see we need to resolve some conflicts, and I have a couple of questions, if you have the time. Also, this is our first adventure into extending the Azure Provider, so please don't hesitate to let us know what else we may need to do. If it helps, I am on the Terraform-Azure Slack channel.

You are correct about the general aspect of the healthcare services client vs. being FHIR specific. Since this is still in preview mode, there hasn't been a lot of information released as to what other kinds of services (and their requirements) will be offered. Since our organization is primarily interested in the FHIR API, we made our changes with this in mind. We will rename the resources, and do you think we need to do anything else in that regard? At least for now?

Also, the Azure healthcare apis package is still in preview mode, and is projected to be released this quarter (2019Q3). We do not know what, if any, changes will be made. Is it better to wait until the formal Azure release, or can we look to merge this now and update it afterwards?

Thank you so much,
-Lance

…rovider-azurerm into fhir-api-rm-tests

katbyte

Hi @kutsovmqs,

Thank you for the revisions. This is looking pretty good now. I've left some more comments inline with the biggest change being the switch to use TypeSets. Sorry i missed that earlier, this change is so that if a user rearranges the items it won't trigger a plan.

katbyte · 2019-10-17T14:20:29Z

azurerm/data_source_healthcare_service_test.go

+
+func TestAccAzureRMDataSourceHealthcareService(t *testing.T) {
+	dataSourceName := "data.azurerm_healthcare_service.test"
+	ri := acctest.RandIntRange(10000, 99999)


I think testacc19101706384399625 is 24 characters 🤔

katbyte · 2019-10-17T14:24:47Z

azurerm/data_source_healthcare_service.go

+							Elem: &schema.Schema{
+								Type:         schema.TypeString,
+								ValidateFunc: validate.NoEmptyStrings,
+							},


Type int shouldn't have Elem set?

azurerm/data_source_healthcare_service.go

website/docs/d/healthcare_service.html.markdown

website/docs/r/healthcare_service.html.markdown

katbyte · 2019-10-23T09:28:39Z

website/docs/r/healthcare_service.html.markdown

+* `cosmosdb_offer_throughput` - The provisioned throughput for the backing database. Range of `400`-`1000`. Defaults to `400`.
+* `tags` - A mapping of tags to assign to the resource.
+
+Authentication Configuration


Could we use the same pattern here as the rest of the provider docs? ie:

* `identity` - An `identity` block as defined below, which contains the Managed Service Identity information for this App Service. --- A `identity` block exports the following: * `principal_id` - The Principal ID for the Service Principal associated with the Managed Service Identity of this App Service.

The resource documentation has been updated with this suggestion and more.

azurerm/data_source_healthcare_service_test.go

Co-Authored-By: kt <[email protected]>

Some typing changes to TypeSet. Minor refactoring. Documentation updates.

MedalLance · 2019-10-24T05:01:38Z

Hi @katbyte, thank you for the review. We've made some more changes to accommodate almost all of your requests and suggestions. There is one request for capitalization that we can not accommodate, and that is mentioned above. Also, could you please expand on your comments regarding the random int. All acceptance tests pass with the code as it is now.

We'd love to get this into 1.36, and I'll be available to work on any additional requests. I am on the Azure Provider Slack channel (MedalLance) if you'd like to chat. Thank you very much.

katbyte

Thanks for the revisions @kutsovmqs,

Not to much left now, look forward to getting this merged once the comments i've left inline are addressed.

katbyte · 2019-10-24T09:48:11Z

website/docs/r/healthcare_service.html.markdown

+A `cors_configuration` exports the following:
+
+* `allow_credentials` - (Boolean) If credentials are allowed via CORS.
+* `allowed_headers` - A set of headers allowed via CORS.
+* `allowed_methods` - The methods allowed via CORS.
+* `allowed_origins` - A set of origins allowed via CORS.
+* `max_age_in_seconds` - The max age allowed via CORS.


as these are set above we don't need to add them to the list of exported properties

katbyte · 2019-10-24T09:49:02Z

website/docs/r/healthcare_service.html.markdown

+
+The following arguments are supported:
+
+* `name` - (Required) The name of the service instance. Used for service endpoint, must be unique within the audience and can contain only lowercase letters, numbers and the '-' character.


We usually don't detail the valid names here, instead we catch them with validation functions.

katbyte · 2019-10-24T09:49:29Z

website/docs/r/healthcare_service.html.markdown

+* `access_policy_object_ids` - A set of Azure object id's that are allowed to access the Service.
+* `authentication_configuration` - An `authentication_configuration` block as defined below.
+* `cors_configuration` - A `cors_configuration` object as defined below.
+* `cosmosdb_throughput` - The provisioned throughput for the backing database.
+* `kind` - The kind or type of Healthcare Service defined.
+* `location` - The Azure Region where the Service should was created.
+* `name` - The account name of the Service.
+* `resource_group_name` - The resource group in which the Service was created.
+* `tags` - The mapping of tags to assigned to the resource.
+
+---
+An `authentication_configuration` exports the following:
+
+* `audience` - The audience that receives authentication tokens for the service.
+* `authority` - The Azure Active Directory (tenant) that serves as the authentication authority to access the service.
+* `smart_proxy_enabled` - (Boolean) If the 'SMART on FHIR' option is enabled for mobile and web implementations.


These are all assignable and above so we don't need to relist them here

katbyte · 2019-10-24T09:50:03Z

website/docs/d/healthcare_service.html.markdown

+
+~> **Please Note**: Not all locations support this resource. Some are `West US 2`, `North Central US`, and `UK West`. 
+
+* `kind` - The type of the service. i.e.: "fhir", "fhir-Stu3" and "fhir-R4".


We don't usually list possible types in the dat source

Suggested change

* `kind` - The type of the service. i.e.: "fhir", "fhir-Stu3" and "fhir-R4".

* `kind` - The type of the service. i.e.

Removed in commit

katbyte · 2019-10-24T09:50:58Z

website/docs/d/healthcare_service.html.markdown

+* `tags` - A mapping of tags to assign to the resource.
+
+Authentication Configuration
+
+* `authority` - The Azure Active Directory (tenant) that serves as the authentication authority to access the service.
+* `audience` - The intended audience to receive authentication tokens for the service..
+* `smart_proxy_enabled` - Enabled status of the 'SMART on FHIR' option for mobile and web implementations.
+
+CORS Configuration
+
+* `allowed_origins` - The origins to be allowed via CORS.
+* `allowed_headers` - The headers to be allowed via CORS.
+* `allowed_methods` - The methods to be allowed via CORS.
+* `max_age_in_seconds` - The max age to be allowed via CORS.
+* `allow_credentials` - If credentials are allowed via CORS.


Could we make these the same as the resource?

* `authentication_configuration` - An `authentication_configuration` block as defined below. ... --- An `authentication_configuration` exports the following: * `audience` - The audience that receives authentication tokens for the service. * `authority` - The Azure Active Directory (tenant) that serves as the authentication authority to access the service. * `smart_proxy_enabled` - (Boolean) If the 'SMART on FHIR' option is enabled for mobile and web implementations. ...

katbyte · 2019-10-24T09:54:57Z

azurerm/resource_arm_healthcare_service_test.go

+)
+
+func TestAccAzureRMHealthcareService_basic(t *testing.T) {
+	ri := acctest.RandIntRange(10000, 99999)


Could we use the standard tf.AccRandTimeInt here? if need /10 or /100 for it to fit

For this and the data source test, we switched to tf. AccRandTimeInt / 10 and changed the prefix to testacc for a format of testacc19102406571368976, as suggested. Hopefully this will be random enough to be unique and still be 24 chars.

katbyte · 2019-10-24T09:55:09Z

azurerm/data_source_healthcare_service_test.go

+
+func TestAccAzureRMDataSourceHealthcareService(t *testing.T) {
+	dataSourceName := "data.azurerm_healthcare_service.test"
+	ri := acctest.RandIntRange(10000, 99999)


this is still a concern

MedalLance · 2019-10-24T14:39:34Z

Hi @katbyte, I've made the requested changes. Please take a look and let me know you think. Thank you.

katbyte

Thanks for the updates @kutsovmqs! Aside from a few minor changes to the docs which i just pushed this LGTM 👍

ghost · 2019-10-29T10:35:45Z

This has been released in version 1.36.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 1.36.0"
}
# ... other configuration ...

ghost · 2019-11-24T14:50:26Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

MedalLance and others added 3 commits July 29, 2019 12:02

Initial creation of FHIR API Sertvice components. All CRUD operations

d2700d6

have been performed using the Azure SDK for Go and the fiunctionality added to the functions in resource_arm_fhir_api_service. DEVOPS-1201

add tests for fhir services

927edaf

add tests for fhir services

17a9656

ghost added the size/XL label Sep 3, 2019

katbyte changed the title ~~Fhir api rm tests~~ New Resource & DataSource: azurerm_fhir_api_service Sep 3, 2019

katbyte added new-data-source new-resource labels Sep 3, 2019

katbyte reviewed Sep 3, 2019

View reviewed changes

rename fhir service to healthcare service

d49f636

katbyte changed the title ~~New Resource & DataSource: azurerm_fhir_api_service~~ New Resource & DataSource: azurerm_healthcare_service Sep 5, 2019

minor fixes

0402ef1

resolve conflicts

07400ae

katbyte added this to the v1.34.0 milestone Sep 10, 2019

resolve conflicts

4418873

ghost added size/XXL and removed size/XL labels Sep 11, 2019

kutsovmqs and others added 2 commits September 11, 2019 16:08

resolve conflicts

2cfa2dd

Merge branch 'master' into fhir-api-rm-tests

84ef840

ghost added size/XS and removed size/XXL labels Sep 11, 2019

kutsovmqs added 2 commits September 11, 2019 16:34

resolve conflicts

65e19c8

Merge branch 'fhir-api-rm-tests' of github.com:medal-labs/terraform-p…

fa9df1c

…rovider-azurerm into fhir-api-rm-tests

ghost added size/XL and removed size/XS labels Sep 11, 2019

resolve conflicts

0438ae3

ghost added size/XXL dependencies and removed size/XL labels Sep 11, 2019

make lint fixes

a0523eb

katbyte requested changes Oct 23, 2019

View reviewed changes

katbyte added the waiting-response label Oct 23, 2019

MedalLance and others added 13 commits October 23, 2019 09:22

Update website/docs/r/healthcare_service.html.markdown

facef7f

Co-Authored-By: kt <[email protected]>

Update azurerm/data_source_healthcare_service_test.go

551e420

Co-Authored-By: kt <[email protected]>

Update website/docs/d/healthcare_service.html.markdown

edbff2f

Co-Authored-By: kt <[email protected]>

Update azurerm/data_source_healthcare_service.go

1077607

Co-Authored-By: kt <[email protected]>

Update azurerm/data_source_healthcare_service.go

fead4ab

Co-Authored-By: kt <[email protected]>

Update azurerm/data_source_healthcare_service.go

451e47d

Co-Authored-By: kt <[email protected]>

Update azurerm/resource_arm_healthcare_service.go

e3ba590

Co-Authored-By: kt <[email protected]>

Update azurerm/resource_arm_healthcare_service.go

a7adf26

Co-Authored-By: kt <[email protected]>

Update website/docs/d/healthcare_service.html.markdown

9be88eb

Co-Authored-By: kt <[email protected]>

Update azurerm/resource_arm_healthcare_service_test.go

66680a6

Co-Authored-By: kt <[email protected]>

Update azurerm/resource_arm_healthcare_service_test.go

d462baf

Co-Authored-By: kt <[email protected]>

Update website/docs/d/healthcare_service.html.markdown

ae378fd

Co-Authored-By: kt <[email protected]>

Requested changes for PR 4221 - 10-23-19

8525a48

Some typing changes to TypeSet. Minor refactoring. Documentation updates.

ghost removed the waiting-response label Oct 24, 2019

change for tflint error

b3ad585

katbyte requested changes Oct 24, 2019

View reviewed changes

Docuemntation and test name generation requested changes

a7a838f

katbyte mentioned this pull request Oct 24, 2019

Support for Azure API for FHIR® #3989

Closed

katbyte added 2 commits October 25, 2019 04:21

Update healthcare_service.html.markdown

603572b

Update healthcare_service.html.markdown

516037e

katbyte approved these changes Oct 25, 2019

View reviewed changes

katbyte merged commit a975734 into hashicorp:master Oct 25, 2019

katbyte added a commit that referenced this pull request Oct 25, 2019

Update CHANGELOG.md to include #4221

a7b106c

ghost locked and limited conversation to collaborators Nov 24, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource & DataSource: azurerm_healthcare_service #4221

New Resource & DataSource: azurerm_healthcare_service #4221

kutsovmqs commented Sep 3, 2019 •

edited by tombuildsstuff

Loading

katbyte left a comment

MedalLance commented Sep 9, 2019

katbyte left a comment

katbyte Oct 17, 2019

katbyte Oct 17, 2019

MedalLance Oct 23, 2019

katbyte Oct 23, 2019

MedalLance Oct 24, 2019

MedalLance commented Oct 24, 2019

katbyte left a comment

katbyte Oct 24, 2019

MedalLance Oct 24, 2019

katbyte Oct 24, 2019

MedalLance Oct 24, 2019

katbyte Oct 24, 2019

MedalLance Oct 24, 2019

katbyte Oct 24, 2019

MedalLance Oct 24, 2019

katbyte Oct 24, 2019

MedalLance Oct 24, 2019

katbyte Oct 24, 2019

MedalLance Oct 24, 2019

MedalLance Oct 24, 2019 •

edited

Loading

katbyte Oct 24, 2019

MedalLance commented Oct 24, 2019

katbyte left a comment

ghost commented Oct 29, 2019

ghost commented Nov 24, 2019


		The following arguments are supported:

		* `name` - (Required) The name of the service instance. Used for service endpoint, must be unique within the audience and can contain only lowercase letters, numbers and the '-' character.


		~> Please Note: Not all locations support this resource. Some are `West US 2`, `North Central US`, and `UK West`.

		* `kind` - The type of the service. i.e.: "fhir", "fhir-Stu3" and "fhir-R4".

New Resource & DataSource: azurerm_healthcare_service #4221

New Resource & DataSource: azurerm_healthcare_service #4221

Conversation

kutsovmqs commented Sep 3, 2019 • edited by tombuildsstuff Loading

katbyte left a comment

Choose a reason for hiding this comment

MedalLance commented Sep 9, 2019

katbyte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MedalLance commented Oct 24, 2019

katbyte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MedalLance Oct 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MedalLance commented Oct 24, 2019

katbyte left a comment

Choose a reason for hiding this comment

ghost commented Oct 29, 2019

ghost commented Nov 24, 2019

kutsovmqs commented Sep 3, 2019 •

edited by tombuildsstuff

Loading

MedalLance Oct 24, 2019 •

edited

Loading