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

update(query): update App Service Not Using Latest TLS Encryption Version query to the latest version #7302

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

anterosilva1985
Copy link
Contributor

Latest TLS Encryption Version is 1.3

Closes #

Reason for Proposed Changes

Proposed Changes

I submit this contribution under the Apache-2.0 license.

Latest TLS Encryption Version is 1.3
@anterosilva1985 anterosilva1985 requested a review from a team as a code owner December 5, 2024 15:04
@github-actions github-actions bot added community Community contribution query New query feature labels Dec 5, 2024
@anterosilva1985 anterosilva1985 changed the title Update query.rego update App Service Not Using Latest TLS Encryption Version query to the latest version Dec 5, 2024
@anterosilva1985 anterosilva1985 changed the title update App Service Not Using Latest TLS Encryption Version query to the latest version update(query): update App Service Not Using Latest TLS Encryption Version query to the latest version Dec 5, 2024
@ArturRibeiro-CX
Copy link
Contributor

Hey @anterosilva1985,
Nice contribution and great update to the query! 🎉

To make the test cases more comprehensive and complete, I would suggest the following changes:

1 - Add a positive expected result (positive2.tf) to signal an invalid configuration;
2 - Update positive_expected_result.json with the new expected result added;
3 - Update negative1.tf modifying the file with the new valid encryption level (1.3);

Negative1.tf update suggestion
resource "azurerm_app_service" "negative1" {
  name                = "example-app-service"
  location            = azurerm_resource_group.example.location
  resource_group_name = azurerm_resource_group.example.name
  app_service_plan_id = azurerm_app_service_plan.example.id

  site_config {
    dotnet_framework_version = "v4.0"
    scm_type                 = "LocalGit"
    min_tls_version = 1.3
  }
}
Additional Positive2.tf result && Updated Positive_expected_result.json suggestion

We could modify the current negative1 file to reflect the new invalid encryption level, such as using a version lower than 1.3 (e.g., encryption: 1.1 and 1.2 with the new query update).
This ensures the query accurately flags cases with unsupported encryption levels and effectively warns new users with the new valid encription level, which needs to be equal or higher than 1.3.

resource "azurerm_app_service" "positive2" {
  name                = "example-app-service"
  location            = azurerm_resource_group.example.location
  resource_group_name = azurerm_resource_group.example.name
  app_service_plan_id = azurerm_app_service_plan.example.id

  site_config {
    dotnet_framework_version = "v4.0"
    scm_type                 = "LocalGit"
    min_tls_version = 1.2
  }
}

Regarding the positive_expected_result file, we could just add a new entry, updating the file as demonstrated below:

[
  {
    "queryName": "App Service Not Using Latest TLS Encryption Version",
    "severity": "MEDIUM",
    "line": 10,
    "fileName": "positive1.tf"
  },
  {
    "queryName": "App Service Not Using Latest TLS Encryption Version",
    "severity": "MEDIUM",
    "line": 10,
    "fileName": "positive2.tf"
  }
]

These are just community suggestions, but they would fix the failing unit tests and, I believe, complete your query update while enhancing the test cases a little bit. 😄
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution query New query feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants