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

feat(azure): Azure new checks related with AKS #3476

Merged
merged 38 commits into from Mar 5, 2024

Conversation

puchy22
Copy link
Contributor

@puchy22 puchy22 commented Mar 1, 2024

Context

Add new checks for Azure that are related with AKS.

Description

Add this checks with his respective unit tests:

  • defender_container_images_scan_enabled
  • defender_container_images_resolved_vulnerabilities
  • aks_clusters_public_access_disabled
  • aks_clusters_created_with_private_nodes
  • aks_network_policy_enabled
  • aks_rbac_enabled

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the provider/azure Issues/PRs related with the Azure provider label Mar 1, 2024
Copy link

codecov bot commented Mar 1, 2024

Codecov Report

Attention: Patch coverage is 94.59459% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 85.93%. Comparing base (3eeca73) to head (d4267e7).
Report is 2 commits behind head on master.

Files Patch % Lines
...rowler/providers/azure/services/aks/aks_service.py 74.19% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3476      +/-   ##
==========================================
+ Coverage   85.83%   85.93%   +0.10%     
==========================================
  Files         662      670       +8     
  Lines       20643    20790     +147     
==========================================
+ Hits        17719    17866     +147     
  Misses       2924     2924              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@puchy22 puchy22 marked this pull request as ready for review March 4, 2024 11:06
@puchy22 puchy22 requested a review from a team as a code owner March 4, 2024 11:06
poetry.lock Outdated
@@ -1,4 +1,4 @@
# This file is automatically @generated by Poetry 1.7.1 and should not be changed by hand.
# This file is automatically @generated by Poetry 1.7.0 and should not be changed by hand.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# This file is automatically @generated by Poetry 1.7.0 and should not be changed by hand.
# This file is automatically @generated by Poetry 1.7.1 and should not be changed by hand.

from prowler.providers.azure.lib.audit_info.audit_info import azure_audit_info
from prowler.providers.azure.services.aks.aks_service import Aks

aks_client = Aks(azure_audit_info)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
aks_client = Aks(azure_audit_info)
aks_client = AKS(azure_audit_info)

report.subscription = subscription_name
report.resource_name = cluster.name
report.resource_id = cluster_id
report.status_extended = f"Network policy enabled for cluster '{cluster.name}' in subscription '{subscription_name}'."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.status_extended = f"Network policy enabled for cluster '{cluster.name}' in subscription '{subscription_name}'."
report.status_extended = f"Network policy is enabled for cluster '{cluster.name}' in subscription '{subscription_name}'."

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the Network Policy name here, please?


if not getattr(cluster, "network_policy", False):
report.status = "FAIL"
report.status_extended = f"Network policy not enabled for cluster '{cluster.name}' in subscription '{subscription_name}'."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.status_extended = f"Network policy not enabled for cluster '{cluster.name}' in subscription '{subscription_name}'."
report.status_extended = f"Network policy is not enabled for cluster '{cluster.name}' in subscription '{subscription_name}'."

@@ -0,0 +1,30 @@
{
"Provider": "azure",
"CheckID": "aks_rbac_enabled",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"CheckID": "aks_rbac_enabled",
"CheckID": "aks_cluster_rbac_enabled",

"CLI": "",
"NativeIaC": "",
"Other": "https://www.trendmicro.com/cloudoneconformity/knowledge-base/azure/AKS/enable-role-based-access-control-for-kubernetes-service.html#",
"Terraform": ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Terraform": ""
"Terraform": "https://docs.bridgecrew.io/docs/bc_azr_kubernetes_2#terraform"

"CLI": "",
"NativeIaC": "",
"Other": "",
"Terraform": ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Terraform": ""
"Terraform": "https://docs.bridgecrew.io/docs/bc_azr_kubernetes_4#terraform"

report.subscription = subscription_name
report.resource_name = cluster.name
report.resource_id = cluster_id
report.status_extended = f"RBAC enabled for cluster '{cluster.name}' in subscription '{subscription_name}'."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.status_extended = f"RBAC enabled for cluster '{cluster.name}' in subscription '{subscription_name}'."
report.status_extended = f"RBAC is enabled for cluster '{cluster.name}' in subscription '{subscription_name}'."


if not cluster.rbac_enabled:
report.status = "FAIL"
report.status_extended = f"RBAC not enabled for cluster '{cluster.name}' in subscription '{subscription_name}'."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.status_extended = f"RBAC not enabled for cluster '{cluster.name}' in subscription '{subscription_name}'."
report.status_extended = f"RBAC is not enabled for cluster '{cluster.name}' in subscription '{subscription_name}'."

clusters.update({subscription_name: {}})

for cluster in clusters_list:
if getattr(cluster, "kubernetes_version", ""):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if getattr(cluster, "kubernetes_version", ""):
if getattr(cluster, "kubernetes_version", None):

!= "Unhealthy"
):
report.status = "PASS"
report.status_extended = f"Azure running container images should have vulnerabilities resolved (powered by Microsoft Defender Vulnerability Management) is enabled in subscription '{subscription_name}'."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.status_extended = f"Azure running container images should have vulnerabilities resolved (powered by Microsoft Defender Vulnerability Management) is enabled in subscription '{subscription_name}'."
report.status_extended = f"Azure running container images do not have unresolved vulnerabilities in subscription '{subscription_name}'."

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This metadata is for the new check defender_container_images_scan_enabled

Comment on lines 22 to 24
if not pricings["Containers"].extensions[
"ContainerRegistriesVulnerabilityAssessments"
]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if not pricings["Containers"].extensions[
"ContainerRegistriesVulnerabilityAssessments"
]:
if not pricings["Containers"].extensions.get(
"ContainerRegistriesVulnerabilityAssessments"
):

Comment on lines 10 to 12
"Containers" in pricings
and "ContainerRegistriesVulnerabilityAssessments"
in pricings["Containers"].extensions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Containers" in pricings
and "ContainerRegistriesVulnerabilityAssessments"
in pricings["Containers"].extensions
"Containers" in pricings

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, review it carefully.

Copy link
Member

@sergargar sergargar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome job!! 🚀

@sergargar sergargar merged commit ddd43ba into prowler-cloud:master Mar 5, 2024
11 checks passed
@puchy22 puchy22 deleted the azure-aks-checks branch March 25, 2024 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
provider/azure Issues/PRs related with the Azure provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants