New Resource azurerm_static_site

[aristosvo]

Fixes #7029

TerraForm Spec

resource "azurerm_static_site" "example" {
  name = "example"
  resource_group_name = "rg"
  location = "West Europe"
  
  github_configuration {
    repo_url          = "https://github.com/aristosvo/azure-static-web-app"
    branch            = "master"
    repo_token        = "xxxxxxxx"

    app_location      = "/"
    api_location      = "/api"
    artifact_location = "dist/angular-basic"
  }
}

Acceptance Tests

--- PASS: TestAccAzureRMStaticSite_basic (168.77s)
--- PASS: TestAccAzureRMStaticSite_requiresImport (171.88s)

[aristosvo]

Implementation of the initial version is done, but to acctest this properly we would need the binary test functionality to generate the GitHub repo.

[jackofallops]

Hi @aristosvo
Thanks for this PR for this new Preview Resource.
It's looking pretty good, and I've left some comments in line below. The key point is that I think the resource can be simplified somewhat by flattening the schema and moving the github_configuration contents to the top level.

We could also do with additional acceptance tests on the resource, in particular an update test, but any other additional appropriate coverage is welcome.

Just to set expectations appropriately...
There is something of a blocking issue on this resource for us. As you probably noticed, every create/destroy cycle leaves behind a GH Action in the branch used with a "pet" name for the deploy, and every one of them is executed every time a commit is made to that branch (or PR to it). In our testing environment this would result in many GH build/deploy actions firing (and failing, and sending emails) after relatively few testing cycles. (to see what I mean, take a look at the actions tab your repo from the tests). I'm going to try to reach out to the service team on this to find out the intent / future of this. Until that blocker is resolved in some way, we'll not be able to merge this.

[jackofallops]

These should probably be an Optional schema item, with a default of Free. Whilst this service is in early preview only Free is selectable in the portal, however, other the API information suggest that other values are possible (or going to be).

[jackofallops]

You shouldn't need to set this, it is usually set by the service.

[jackofallops]

as noted above

Suggested change

Type: &staticSiteType,

[jackofallops]

We should also check if the ID field is empty here

Suggested change

	if read.ID == nil {
	if read.ID == nil || read.ID == "" {

[jackofallops]

Are there any sensible validation steps that can be taken for these items to catch problems at plan stage and prevent issues elsewhere?

[jackofallops]

I believe this resource could be made more simple by moving these items to the top level, rather than having a separate block.
(If there's a reason you feel it should be a block, I think this should have MaxItems: 1)

[jackofallops]

This should probably have a TF prefix, rather than ARM as these are for configuring the client itself, rather than test flags.

Suggested change

	return os.Getenv("ARM_TEST_GITHUB_TOKEN") == ""
	return os.Getenv("TF_ACCTEST_GITHUB_TOKEN") == ""

[jackofallops]

Could we have a test for this function as for other ID type elsewhere in the provider?

[jackofallops]

Hi @aristosvo
I hope you don't mind, I needed to check if this service had been unblocked for us, so I quickly completed the refactor / rework you started to a testable point and fixed the merge conflicts. Sadly we're still not going to be able to unblock this due to the GH actions behaviours being incompatible with the service being managed by terraform. I've reached out to my contacts at Microsoft to see if there's a route forward for this. Any meaningful updates I get I'll post here.

[payamazadi]

would be really excited to see this come through, any updates please @jackofallops ?

meanwhile could you or @aristosvo could you provide any quick documentation on how to include this as a patch for individuals who want to use it? maybe it doesnt make sense for the core/master right now but it doesn't mean other folks couldn't take advantage of it..

[Oceanswave]

Hi @aristosvo
I hope you don't mind, I needed to check if this service had been unblocked for us, so I quickly completed the refactor / rework you started to a testable point and fixed the merge conflicts. Sadly we're still not going to be able to unblock this due to the GH actions behaviours being incompatible with the service being managed by terraform. I've reached out to my contacts at Microsoft to see if there's a route forward for this. Any meaningful updates I get I'll post here.

Without any meaningful updates in a while, I'm wondering if there is an alterative path forward - since

1. This is 'mostly' a blocker only for terraform resource test code due to the static web app resource creation having a side effect of creating a GH Action
2. The Github Action that gets created by the static web app creation is represented by a .yml file with the associated repository
3. The Github repo, branch and token is included as part of the configuration

could the test be modified so that it includes test tear down behaviour to reach out to the defined GH repo/branch/token and remove the .github/workflows/azure-static-web-apps-{{ d.Id() }}.yml file - implementation might look similar to this

[jackofallops]

Hi @Oceanswave
Thanks for the comment and the suggestion. It's an approach we have already considered, but involves vendoring a large dependency for the sake of removing a single file from a repository.
The nightly/regular testing aspect is indeed part of our blocker for proceeding, this makes a lot of noise quite quickly, but this will also translate to similar issue for users employing this resource for non-production type environments (dev / staging / test / other). We're looking at options, but waiting on a few answers from the service team. Ultimately, this is still a preview, so could be subject to significant implementation change right up to GA. It may be that after the answers, we go ahead an include it as is, which some warning(s) around this behaviour and some examples/suggestions of how to stop it getting out of hand (since we'd need to employ this for the acceptance tests also).
fwiw - we're chasing this at every opportunity, as we'd like it in too.

[payamazadi]

thanks @jackofallops . Any updates?

Meanwhile, I managed to get this patched into my local version of Terraform and it works flawlessly. This was a big service to the community @aristosvo and is much appreciated!

[mpetuska]

@payamazadi, could you please briefly outline how you've managed to patch this in locally?

[magodo]

Is that possible to ask the service team provide an option to not create the workflow for users, rather return the content of the yaml file and the secret in the creation response. Then users can use the github_repository_file to manage that workflow file, and use github_actions_secret to manage the secret.

This way we do not need to involve the dependency on github related packages, and since we recently opt in the binary testing framework, it should also works for acctest.

@jackofallops WDYT?

[magodo]

@jackofallops and @aristosvo I've pushed two commits to this PR.

One is to remove the repo-related properties from static site resource (to avoid service manipulate the repo) and adding some comptued properties (to allow users to add the api key and workflow to the repo by their own).
The other one is to merge with the master branch, changing the test style a bit.

I've passed the acctest locally:

💢 TF_ACC=1 go test -v -timeout=180m -run=TestAccAzureStaticSite_ ./...
2021/03/16 14:46:00 [DEBUG] not using binary driver name, it's no longer needed
2021/03/16 14:46:00 [DEBUG] not using binary driver name, it's no longer needed
=== RUN   TestAccAzureStaticSite_basic
=== PAUSE TestAccAzureStaticSite_basic
=== RUN   TestAccAzureStaticSite_requiresImport
=== PAUSE TestAccAzureStaticSite_requiresImport
=== CONT  TestAccAzureStaticSite_basic
=== CONT  TestAccAzureStaticSite_requiresImport
--- PASS: TestAccAzureStaticSite_basic (221.86s)
--- PASS: TestAccAzureStaticSite_requiresImport (240.79s)

@jackofallops Please take another look, thank you!

[tombuildsstuff]

@magodo without the ability to connect this resource to a repository, the provisioned Static Site will be non-functional (e.g. no site will be deployed, which is the point of this service) - so this still appears to need service changes to be usable, or is there some other automated way of connecting the repo?

[magodo]

@tombuildsstuff You can use the github terraform provider to do that, like this: https://gist.github.com/magodo/287009b96ebfa6b8c173e62d6907fa0f.
Note that you'd need to ensure the github access token has the workflow permission, as it will be used to create a workflow file in that repo.
After above resources are provisioned, the github action will be triggered, which will in turn notify the static site to pull the repo and do the deployment.

[aristosvo]

Looks good within current limitations from my point of view! I'd probably add your example code to the documentation to have clear instructions how to use this resource, as without the GitHub configuration functionality is indeed limited.

Just thinking out loud, but maybe we'd even go as far as adding that scenario as an acceptance test? 😇 Binary testing is awesome 🤩

[magodo]

@aristosvo Good point! The reason why I didn't add my example to the document is because currently there is a bug in the github provider, which will cause the provider panic on file creation. Anyway, I've added an example for this.

[scuderig]

Any update on this? The service has been announced as GA but we still haven’t got a way to provision using IaC..

[dirien]

@scuderig, asking the same question. Looking forward to use this via TF too.

[katbyte]

Aside from one comment looks good to me 👍

[ghost]

This has been released in version 2.60.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 = "~> 2.60.0"
}
# ... other configuration ...

[github-actions]

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.