fix(ecs-service): #624 fix description of ecs service cluster field

[haarchri]

Description of your changes

fixed the description for ecs_service cluster field - we need the cluster name / short-id instead of ARN - ref & selector working as expected as mentioned by @dverveiko in #624

Fixes #624

I have:

• Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

applied new example:

NAME                                                 READY   SYNCED   EXTERNAL-NAME             AGE
cluster.ecs.aws.upbound.io/example-cluster-service   True    True     example-cluster-service   4m2s

NAME                                         READY   SYNCED   EXTERNAL-NAME     AGE
service.ecs.aws.upbound.io/example-service   True    True     example-service   4m2s

NAME                                                           READY   SYNCED   EXTERNAL-NAME   AGE
taskdefinition.ecs.aws.upbound.io/example-service-definition   True    True     sampleservice   4m2s

[ytsarev]

Is that actually correct? While selectors will work the actually field will take and ARN as an input https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ecs_service#cluster

[haarchri]

@ytsarev check #624 the examples in the comments - you see that cluster is everytime the name instead of the arn

[ytsarev]

Got it, thanks for clarification.

In this case, we need to extend uptest suite with an example symmetrical to https://github.com/upbound/provider-aws/blob/main/examples/ecs/service.yaml but with using an explicit cluster field name. This way we will deterministically prove that the Name/Non-ARN format works end-to-end.

Could you please add extend the PR with this additional example to cover this case?

[haarchri]

I will add it

[haarchri]

added example and tested:

kubectl get managed
Warning: BucketPolicy has been deprecated. Use spec.forProvider.policy in Bucket instead.
NAME                                                 READY   SYNCED   EXTERNAL-NAME             AGE
cluster.ecs.aws.upbound.io/example-cluster-service   True    True     example-cluster-service   4m2s

NAME                                         READY   SYNCED   EXTERNAL-NAME     AGE
service.ecs.aws.upbound.io/example-service   True    True     example-service   4m2s

NAME                                                           READY   SYNCED   EXTERNAL-NAME   AGE
taskdefinition.ecs.aws.upbound.io/example-service-definition   True    True     sampleservice   4m2s

[ytsarev]

/test-examples="examples/ecs/service-static-cluster.yaml"

[ytsarev]

/test-examples="examples/ecs/service-static-cluster.yaml"

[sergenyalcin]

Thanks @haarchri LGTM! From my side it is good to go if @ytsarev also thinks on this way.

[ytsarev]

@sergenyalcin all good in general. I still fail to understand why we need manual intervention annotation on something that proved to be automatically testable by uptest :) If somebody can explain me that simple thing I am totally happy with the merge :)

[ytsarev]

Ok, @sergenyalcin managed to fix my understanding. We are still skipping skipping Service involved with underlying https://github.com/upbound/uptest/blob/main/internal/prepare.go#L103
and the rest of the resources in example are just getting successfully CRUDed by uptest.
Sorry fo confusion, LGTM