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

waiters with ordering comparators (<, <=, >, >=) do not work #2983

Closed
lucix-aws opened this issue Jan 23, 2025 · 1 comment · Fixed by #2984
Closed

waiters with ordering comparators (<, <=, >, >=) do not work #2983

lucix-aws opened this issue Jan 23, 2025 · 1 comment · Fixed by #2984
Assignees
Labels
bug This issue is a bug. p1 This is a high priority issue

Comments

@lucix-aws
Copy link
Contributor

lucix-aws commented Jan 23, 2025

Any waiter with a jmespath comparison expression using <, <=, >, or >= is broken, because ordered comparison in go-jmespath itself appears to be broken:

package main

import (
	"fmt"

	"github.com/aws/aws-sdk-go-v2/aws"
	"github.com/aws/aws-sdk-go-v2/service/autoscaling/types"
	jmespath "github.com/jmespath-community/go-jmespath"
)

func main() {
	out := types.AutoScalingGroup{
		MinSize: aws.Int32(2),
		MaxSize: aws.Int32(7),
	}

	pathValue, err := jmespath.Search("MaxSize > MinSize", out)
	if err != nil {
		panic(err)
	}

	fmt.Println("go-jmespath gives us", pathValue)
}
go-jmespath gives us <nil>

The following service models contain affected paths: cloudwatch, ec2, and auto-scaling. Fortunately EC2 is not actually broken because we previously migrated this to generated waiters.

[main:~/git/aws-sdk-go-v2/codegen/sdk-codegen/aws-models]$ rg '"path":.*>'
cloudwatch.json
1215:                                        "path": "length(MetricAlarms[]) > `0`",
1230:                                        "path": "length(CompositeAlarms[]) > `0`",

ec2.json
33210:                                        "path": "length(Images[]) > `0`",
34527:                                        "path": "length(Reservations[]) > `0`",
34792:                                        "path": "length(InternetGateways[].InternetGatewayId) > `0`",
35564:                                        "path": "length(KeyPairs[].KeyName) > `0`",
39460:                                        "path": "length(SecurityGroups[].GroupId) > `0`",
56589:                                        "path": "length(PasswordData) > `0`",

auto-scaling.json
4177:                                        "path": "length(AutoScalingGroups) > `0`",
4187:                                        "path": "length(AutoScalingGroups) > `0`",
4202:                                        "path": "contains(AutoScalingGroups[].[length(Instances[?LifecycleState=='InService']) >= MinSize][], `false`)",
4212:                                        "path": "contains(AutoScalingGroups[].[length(Instances[?LifecycleState=='InService']) >= MinSize][], `false`)",
4227:                                        "path": "length(AutoScalingGroups) > `0`",
4237:                                        "path": "length(AutoScalingGroups) > `0`",
[main:~/git/aws-sdk-go-v2/codegen/sdk-codegen/aws-models]$ rg '"path":.*>='
auto-scaling.json
4202:                                        "path": "contains(AutoScalingGroups[].[length(Instances[?LifecycleState=='InService']) >= MinSize][], `false`)",
4212:                                        "path": "contains(AutoScalingGroups[].[length(Instances[?LifecycleState=='InService']) >= MinSize][], `false`)",
[main:~/git/aws-sdk-go-v2/codegen/sdk-codegen/aws-models]$ rg '"path":.*<'
[main:~/git/aws-sdk-go-v2/codegen/sdk-codegen/aws-models]$ rg '"path":.*<='

This means that these waiters in cloudwatch and autoscaling are still broken today.

Fortunately we do have a path forward here in migrating to generated waiters. See #2977.

@lucix-aws lucix-aws added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. p1 This is a high priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Jan 23, 2025
@lucix-aws lucix-aws self-assigned this Jan 23, 2025
Copy link

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p1 This is a high priority issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant