paloaltonetworks / pcs-sizing-scripts Goto Github PK

Please can we have resource counts for ELBs and EKS clusters?

I see code for listing the EKS clusters in the resource-count-aws.sh script but no flag for it. Same for the ELBs as well.

Thank you.

Describe the bug

When run using org mode, the first member account will be scanned. All of the other accounts will fail.

Expected behavior

It should scan all of the member accounts

Current behavior

Warning: Failed to assume role into Member Account …, skipping ...

If you remove the 2>/dev/null which discards the error output from the aws sts assume-role command you'll see this additional detail:

Unable to locate credentials. You can configure credentials by running "aws configure".

Possible solution

Reviewing the code from #2, I believe the problem is related to the way assume_role() overwrites its current credentials with the assumed role credentials, which allows that account to be scanned but then causes all subsequent accounts to fail since you'd need to call sts:AssumeRole using the original credentials.

Steps to reproduce

1. Run resource-count-aws.sh org with credentials in the Organization master account for an IAM user which has permission to assume the OrganizationAccountAccessRole in each member account.

Current score: 105
Target threshold: 100
Total possible: 110

Is your feature request related to a problem?

Markdown is easier to edit!

Describe the bug

resource-count-aws.sh

If there is an AMI launch Index, the EC2 count is different.

About AMI Lunch Index.
https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/AMI-launch-index-examples.html

Expected behavior

Multiple EC2 instances in the AMI Launch Index are counted.

Current behavior

Multiple EC2 instances in the AMI Launch Index are not counted.

Possible solution

I am not good at jq.
For example, I think it is possible to implement the following using grep and wc.

resource-count-aws.sh
line 293
RESOURCE_COUNT=$(aws_ec2_describe_instances "${i}" | grep InstanceId | wc -l' 2>/dev/null)

Is your feature request related to a problem?

It seems we are only checking for elb, and not including elbv2 as part of our API queries.

Describe the solution you'd like

I would like us to include support for aws elbv2 describe-load-balancers.

Describe alternatives you've considered

Asking the client for how many ELBs they are using.

Additional context

This issue was identified by a client during a PoC. They ran the script, and it returned zero for the ELB service. However, the client knows that they have several ELBs.

Describe the bug

In cwp mode, ECS Task is not counted correctly.
The number of ECS Fargate Tasks cannot be obtained correctly in aws-resource-count.sh with cwp option.

Expected behavior

To obtain the correct ECS cluster name and count the number of individual Fargate tasks.

Current behavior

FargateTask per cluster is not get correctly.

Possible solution

Modify the script function "get_ecs_fargate_task_count()".
For example, how about modifying the following?

https://github.com/PaloAltoNetworks/pcs-sizing-scripts/blob/main/aws/resource-count-aws.sh#L241-L253

  **ECS_FARGATE_CLUSTERS=($(aws_ecs_list_clusters "${REGION}" | jq -r '.clusterArns[]' 2>/dev/null))**

  XIFS=$IFS
  # shellcheck disable=SC2206
  **IFS=$'\n' ECS_FARGATE_CLUSTERS_LIST=(${ECS_FARGATE_CLUSTERS[@]})**
  IFS=$XIFS

  ECS_FARGATE_TASK_LIST_COUNT=0
  RESULT=0

  for CLUSTER in "${ECS_FARGATE_CLUSTERS_LIST[@]}"
  do
    **ECS_FARGATE_TASK_LIST_COUNT=($(aws_ecs_list_tasks "${REGION}" "${CLUSTER}" | jq -r '[.taskArns[]] | length' 2>/dev/null))**

Steps to reproduce

./resource-count-aws.sh cwp

Describe the bug

If an AWS account within an organization begins with a zero, it reports an error at line 190 in the aws/resource-count-aws.sh.as follows:
./resource-count-aws.sh: line 190: [[: 012345678983: value too great for base (error token is "012345678983")

Expected behavior

Value is the test should be their string equivalent.

Current behavior

./resource-count-aws.sh: line 190: [[: 012345678983: value too great for base (error token is "012345678983")

Possible solution

Remove the double bracket test and use single brackets. See https://stackoverflow.com/questions/24777597/value-too-great-for-base-error-token-is-08

Steps to reproduce

At line 189, insert something like: ACCOUNT_ID=012345678983

Screenshots

Context

Your Environment

Is your feature request related to a problem?

Ease of use

Describe the solution you'd like

The AWS sizing script should have a "--compute" flag rather than be 2 separate scripts

Describe alternatives you've considered

There are currently 2 scripts as a workaround

Additional context

Describe the bug

On testing, I found there is a bug in Aliyun CLI which affects the sizing tool. With the bug, the sizing script would just return all 0 in the counting.

Expected behavior

The Aliyun Sizing Script is reading the "Total Count" attribute from the json obtained by "aliyun ecs describeinstances". The "Total Count" should report the total ECS (VM) instances number.

Current behavior

But perhaps an API bug from Alicloud side, the "Total Count" is always 0 even there are vm listed.

Possible solution

Amend the script function "get_instance_count()", skip relying on the "Total Count" attribute. Instead, add the new lines to count the instances number instead.

get_instance_count() {
COUNT=0
RESULT=$(abc_compute_instances_list "${1}")
// COUNT=$(echo "${RESULT}" | jq -r '.TotalCount' 2>/dev/null) //commented this original line
INSTANCES=($(echo "${RESULT}" | jq -r '.Instances.Instance[].InstanceId' 2>/dev/null)) //add new line 1
COUNT=$((COUNT + ${#INSTANCES[@]})) //add new line 2
echo "${COUNT}"
}