Add RateLimit config to serviceDefaults

[kisunji]

Changes proposed in this PR:

• Adds the RateLimits field to serviceDefaults CRD to match the addition in CE commit consul#18583

How I've tested this PR:

• Added unit tests

How I expect reviewers to test this PR:

Checklist:

• Tests added
• CHANGELOG entry added

[lkysow]

Just a driveby but I think we used to also try and exercise new fields in our acceptance tests?

[cthain]

I agree with Luke that we should have k8s acceptance tests for rate limiting. It seems reasonable to do that in a follow up PR.

[wilkermichael]

Another drive by, haven't had a chance to fully review. But did you try creating the resource locally as part of your changes? Just to verify it exists properly in Consul?

[kisunji]

Another drive by, haven't had a chance to fully review. But did you try creating the resource locally as part of your changes? Just to verify it exists properly in Consul?

It's not a new resource but a field added to service-defaults. In an acceptance test I added an assertion that the field exists with the right value from the fixture yaml.

I'll spend time writing a proper acceptance test eventually. I need to shift focus to some multiport/v2 work

[wilkermichael]

Great work on this @kisunji, a few minor comments. I also just want to double check some of the verification logic around including 0 as a valid value.

[wilkermichael]

I know you didn't add these, but I think these should these just be removed? There's no todo comment or anything associated with them 🤷

[wilkermichael]

Another drive by, haven't had a chance to fully review. But did you try creating the resource locally as part of your changes? Just to verify it exists properly in Consul?

It's not a new resource but a field added to service-defaults. In an acceptance test I added an assertion that the field exists with the right value from the fixture yaml.

I'll spend time writing a proper acceptance test eventually. I need to shift focus to some multiport/v2 work

Did you test that locally? The acceptance tests in the pipeline are currently not working, but they should work locally if you build dataplane yourself.

[wilkermichael]

Thanks for addressing my comments!