-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Allow java_version 21 for app services. fixes #25490 #26304
base: main
Are you sure you want to change the base?
Conversation
cfe59c8
to
06dceef
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @computerlove! Thanks for this PR but we've got some failing tests because the new values wasn't added to the correct schema or the tests aren't testing the right resource.
=== RUN TestAccAppService_windowsJava21Java
=== PAUSE TestAccAppService_windowsJava21Java
=== CONT TestAccAppService_windowsJava21Java
testcase.go:113: Step 1/2 error: Error running pre-apply refresh: exit status 1
Error: expected site_config.0.java_version to be one of ["1.7" "1.8" "11"], got 21
with azurerm_app_service.test,
on terraform_plugin_test.tf line 48, in resource "azurerm_app_service" "test":
48: java_version = "21"
--- FAIL: TestAccAppService_windowsJava21Java (11.10s)
=== CONT TestAccAppService_windowsJava17Java
testcase.go:113: Step 1/2 error: Error running pre-apply refresh: exit status 1
Error: expected site_config.0.java_version to be one of ["1.7" "1.8" "11"], got 17
with azurerm_app_service.test,
on terraform_plugin_test.tf line 48, in resource "azurerm_app_service" "test":
48: java_version = "17"
--- FAIL: TestAccAppService_windowsJava17Java (11.67s)
@mbfrahry When I update the conflict in CHANGELOG.md, should the change be amended and force pushed, or is a merge commit OK? |
@computerlove, we take care of updating the CHANGELOG.md after a PR has been merged, so if there are any changes in there they should be removed. We recommend rebasing your changes on top of main, but merging changes from main into your branch is also acceptable. |
Ok, I have rebased and pushed with not changes to CHANGELOG.md. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @computerlove! I think we're also missing a test for linux function app?
Yeah, looks like it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the tests look good, just one is failing, the Linux Web App test needs to have the check updated
…#25490, hashicorp#24754 Signed-off-by: Marvin B. Lillehaug <marvin.lillehaug@nte.no>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like we have a test failure here
------- Stdout: -------
=== RUN TestAccLinuxWebApp_withJre21Java
=== PAUSE TestAccLinuxWebApp_withJre21Java
=== CONT TestAccLinuxWebApp_withJre21Java
testcase.go:130: Step 1/3 error: After applying this test step, the non-refresh plan was not empty.
stdout:
Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
~ update in-place
Terraform will perform the following actions:
# azurerm_linux_web_app.test will be updated in-place
~ resource "azurerm_linux_web_app" "test" {
id = "/subscriptions/*******/resourceGroups/acctestRG-240815145037738884/providers/Microsoft.Web/sites/acctestWA-240815145037738884"
name = "acctestWA-240815145037738884"
# (20 unchanged attributes hidden)
~ site_config {
# (24 unchanged attributes hidden)
~ application_stack {
+ java_version = "21"
# (2 unchanged attributes hidden)
}
}
}
Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccLinuxWebApp_withJre21Java (250.56s)
FAIL
Yeah, no idea what that means. |
Community Note
Description
Add Java 21 as a allowed value for webapp services.
PR Checklist
Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_linux_web_app
- Support Java 21. (#25490)azurerm_windows_web_app
- Support Java 21. (#25490)azurerm_linux_function_app
- Support Java 21. (#24754)azurerm_windows_function_app
- Support Java 21. (#24754)This is a (please select all that apply):
Related Issue(s)
Fixes #25490, fixes #24754