-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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(arm): ARM AppServiceInstanceMinimum - CKV_AZURE_212 #6502
feat(arm): ARM AppServiceInstanceMinimum - CKV_AZURE_212 #6502
Conversation
def scan_resource_conf(self, conf: Dict[str, Dict[str, Dict[str, int]]]) -> CheckResult: | ||
if "properties" in conf: | ||
if "siteConfig" in conf["properties"]: | ||
if "numberOfWorkers" in conf["properties"]["siteConfig"]: |
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.
Please verify siteConfic
before accessing it, you can see the dogFood test is failing on it.
from checkov.common.models.enums import CheckCategories, CheckResult | ||
|
||
|
||
class AppServiceSlotMinTLS(BaseResourceValueCheck): |
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.
Please remove this check from your PR, we already have this check for those resources in CKV_AZURE_15
& CKV_AZURE_145
|
||
def scan_resource_conf(self, conf: Dict[str, Dict[str, Dict[str, int]]]) -> CheckResult: | ||
if "properties" in conf: | ||
if "siteConfig" in conf["properties"] and conf["properties"]["siteConfig"] is not None: |
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.
consider working with get
instead, it is less complex and more readable. If the key doesn't exists it will return None
.
if "siteConfig" in conf["properties"] and conf["properties"]["siteConfig"] is not None: | |
if conf.get("properties").get("siteConfig") is not None: |
…o#6502) * feat(ARM): AppServiceSetHealthCheck CKV_AZURE_213 * feat(arm) CKV_AZURE_154 * feat(arm) CKV_AZURE_212 * type return * verify siteConfig * remove check AppServiceSlotMinTLS * changed line 23 * Update checkov/arm/checks/resource/AppServiceInstanceMinimum.py --------- Co-authored-by: Rachel <bb50305030@gmail.com> Co-authored-by: ChanochShayner <57212002+ChanochShayner@users.noreply.github.com>
…o#6502) * feat(ARM): AppServiceSetHealthCheck CKV_AZURE_213 * feat(arm) CKV_AZURE_154 * feat(arm) CKV_AZURE_212 * type return * verify siteConfig * remove check AppServiceSlotMinTLS * changed line 23 * Update checkov/arm/checks/resource/AppServiceInstanceMinimum.py --------- Co-authored-by: Rachel <bb50305030@gmail.com> Co-authored-by: ChanochShayner <57212002+ChanochShayner@users.noreply.github.com>
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Description
Added new policy for -
AppServiceInstanceMinimum - CKV_AZURE_212
Fixes # (issue)
New/Edited policies (Delete if not relevant)
Description
policy to ensure App Service has a minimum number of instances for failover
Fix
How does someone fix the issue in code and/or in runtime?
Checklist: