Skip to content
This repository has been archived by the owner on Jan 23, 2025. It is now read-only.

added autoscaling adapter #1153

Merged
merged 16 commits into from
Apr 21, 2023

Conversation

realwebdev
Copy link
Contributor

No description provided.

@CLAassistant
Copy link

CLAassistant commented Jan 27, 2023

CLA assistant check
All committers have signed the CLA.

# provider: aws
# service: autoscaling
# severity: LOW
# short_code: enable-public-access
Copy link
Contributor

Choose a reason for hiding this comment

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

the short code needs to be updated here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

short_code updated

@github-actions
Copy link

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the stale Stale issues will be closed within 7 days of this label being assigned label Mar 24, 2023
@realwebdev
Copy link
Contributor Author

realwebdev commented Apr 10, 2023

@giorod3 review the latest changes in the PR

@github-actions github-actions bot removed the stale Stale issues will be closed within 7 days of this label being assigned label Apr 10, 2023
@realwebdev
Copy link
Contributor Author

review required @CLAassistant @giorod3

@simar7
Copy link
Member

simar7 commented Apr 18, 2023

@realwebdev looks like your PR cannot be merged as there are conflicts with the master branch. Can you resolve them?

@realwebdev
Copy link
Contributor Author

@realwebdev looks like your PR cannot be merged as there are conflicts with the master branch. Can you resolve them?

I have update this branch with master. I don't see conlict anymore. @simar7

@@ -0,0 +1,13 @@

Ensures all autoscaling groups contain at least 1 instance..
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
Ensures all autoscaling groups contain at least 1 instance..
Ensures all autoscaling groups contain at least 1 instance.

Copy link
Member

Choose a reason for hiding this comment

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

You'll probably need to fix this in the rule itself and not in the generated markdown.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved

@@ -0,0 +1,13 @@

Ensures all Auto Scaling groups have ELB health check active..
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
Ensures all Auto Scaling groups have ELB health check active..
Ensures all Auto Scaling groups have ELB health check active.

Copy link
Member

Choose a reason for hiding this comment

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

You'll probably need to fix this in the rule itself and not in the generated markdown.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved

}

var AVZone []types.StringValue
for _, AV := range autoscalingapi.AvailabilityZones {
Copy link
Member

Choose a reason for hiding this comment

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

variable names are usually lowercase in Go.

Suggested change
for _, AV := range autoscalingapi.AvailabilityZones {
for _, av := range autoscalingapi.AvailabilityZones {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved

}

var InsList []autoscaling.InstanceList
for _, IL := range autoscalingapi.Instances {
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved

}

func (a *adapter) getLaunchConfigurations() ([]autoscaling.LaunchConfigurations, error) {
a.Tracker().SetServiceLabel(" Launch Configurations...")
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
a.Tracker().SetServiceLabel(" Launch Configurations...")
a.Tracker().SetServiceLabel("Launch Configurations...")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do you track this mistake this small? I can use it before pushing code. resolved

}

func (a *adapter) getNotificationConfigurations() ([]autoscaling.NotificationConfigurations, error) {
a.Tracker().SetServiceLabel(" Notificaiton Configurations...")
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
a.Tracker().SetServiceLabel(" Notificaiton Configurations...")
a.Tracker().SetServiceLabel("Notificaiton Configurations...")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved

Metadata: r.Metadata(),
Name: r.GetStringProperty("AutoScalingGroupName"),
AvaiabilityZone: getAvailabilityZone(r),
Instances: nil,
Copy link
Member

Choose a reason for hiding this comment

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

Is this always nil? If so why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

corrected it.

return avaiabilityZone
}

for _, AZ := range AvaiabilityZoneList.AsList() {
Copy link
Member

Choose a reason for hiding this comment

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

lowercase variable name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also corrected spellings


func getAvailabilityZone(r *parser.Resource) (avaiabilityZone []types.StringValue) {

AvaiabilityZoneList := r.GetProperty("AvailabilityZones")
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
AvaiabilityZoneList := r.GetProperty("AvailabilityZones")
AvailabilityZoneList := r.GetProperty("AvailabilityZones")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

solved spellings mistake

Comment on lines 114 to 116
if Tag.IsNil() || Tag.IsNotNil() {
return tags
}
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean to say

Suggested change
if Tag.IsNil() || Tag.IsNotNil() {
return tags
}
if Tag.IsNil() || Tag.IsNotListl() {
return tags
}

Otherwise, wouldn't this be always true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

return tags
}

for _, TG := range Tag.AsList() {
Copy link
Member

Choose a reason for hiding this comment

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

lower case variable name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved

Metadata: resource.GetMetadata(),
Name: nameVal,
AvaiabilityZone: AZones,
Instances: nil,
Copy link
Member

Choose a reason for hiding this comment

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

ditto as well for instances being nil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used to place nil where I was unables to find values. Here in instances adapter need instances id and and metadata. but there were no instances id in the documentation. but I change that function with unresolveable strings and metadata? is this the way?

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 share a link to the documentation?

Copy link
Contributor Author

@realwebdev realwebdev Apr 20, 2023

Choose a reason for hiding this comment

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

userDataAttr := resource.GetAttribute("user_data")
userDataVal := userDataAttr.AsStringValueOrDefault("", resource)

iamInstanceProfileAttr := resource.GetAttribute(("iam_instance_profile"))
Copy link
Member

Choose a reason for hiding this comment

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

redundant parenthesis?

Suggested change
iamInstanceProfileAttr := resource.GetAttribute(("iam_instance_profile"))
iamInstanceProfileAttr := resource.GetAttribute("iam_instance_profile")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

I see you tried to resolve some issues @realwebdev but did you forget to push your changes? I don't see any new commits.

Yes I haven't pushed the code. will test it then push the changes

Comment on lines 28 to 29
availability_zones = "us-east-1a"


}
Copy link
Member

Choose a reason for hiding this comment

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

remove extra space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved

@@ -0,0 +1,25 @@
# METADATA
# title: "ASG Multiple AZ"
# description: "Ensures that ASGs are created to be cross-AZ for high availability."
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
# description: "Ensures that ASGs are created to be cross-AZ for high availability."
# description: "Ensures that ASGs are created to be cross-AZ for high availability"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


deny[res] {
group := input.aws.autoscaling.autoscalinggroupslist[_]
count(group.avaiabilityzone) <= 1
Copy link
Member

Choose a reason for hiding this comment

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

this field has a typo, should be fixed everywhere

Suggested change
count(group.avaiabilityzone) <= 1
count(group.availabilityzone) <= 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, there were spelling mistakes. I have corrected. you have mentioned here.

@@ -0,0 +1,26 @@
# METADATA
# title: "ELB Health Check Active"
# description: "Ensures all Auto Scaling groups have ELB health check active.."
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
# description: "Ensures all Auto Scaling groups have ELB health check active.."
# description: "Ensures all Auto Scaling groups have ELB health check active"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -0,0 +1,25 @@
# METADATA
# title: "Empty AutoScaling Group"
# description: "Ensures all autoscaling groups contain at least 1 instance.."
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
# description: "Ensures all autoscaling groups contain at least 1 instance.."
# description: "Ensures all autoscaling groups contain at least 1 instance"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@simar7 simar7 left a comment

Choose a reason for hiding this comment

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

thanks left some comments

@simar7
Copy link
Member

simar7 commented Apr 20, 2023

I see you tried to resolve some issues @realwebdev but did you forget to push your changes? I don't see any new commits.

@simar7 simar7 force-pushed the aws/autoscaling_policies branch from 816568c to 7cc92cc Compare April 21, 2023 22:52
Signed-off-by: Simar <[email protected]>
@simar7 simar7 force-pushed the aws/autoscaling_policies branch from c01528e to b123055 Compare April 21, 2023 23:07
Signed-off-by: Simar <[email protected]>
@simar7 simar7 self-requested a review April 21, 2023 23:50
@simar7 simar7 merged commit 124ecc6 into aquasecurity:master Apr 21, 2023
simar7 added a commit that referenced this pull request Apr 27, 2023
@simar7
Copy link
Member

simar7 commented Apr 28, 2023

This PR adds a new service, which requires an update of cloud schema. As the bundle that gets created with this PR now includes a brand new service, older versions of Trivy cannot parse these newer policies (this is expected).

The way to proceed here will be to update the bundle version of defsec policies. This will enable newer Trivy versions to consume this new bundle but at the same time, older versions of Trivy will still be able to use the older bundles (without newer services).

simar7 added a commit that referenced this pull request May 3, 2023
simar7 added a commit that referenced this pull request May 4, 2023
* Revert "added autoscaling adapter (#1153)"

This reverts commit 124ecc6.

* Revert "add: multiple adapters added(kendra, kinesis, kinesisvideo, proton, q… (#1227)"

This reverts commit 1a1ff87.

* Revert "added in adapters of codebuild (#1184)"

This reverts commit ff5de60.

* Revert "add: lambda adapter (#1166)"

This reverts commit 32da643.

* Revert "added in adapters of mq and msk (#1218)"

This reverts commit 1169455.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants