S3 ACK - Unsupported Argument

[stanenbaum-va]

Describe the bug After applying a bucket resource, the controller logs show the following error

2022-07-27T20:52:34.160Z        ERROR   controller.bucket       Reconciler error        {"reconciler group": "s3.services.k8s.aws", "reconciler kind": "Bucket", "name": "my-ack-s3-bucket", "namespace": "s3-test", "error": "UnsupportedArgument: The request contained an unsupported argument.\n\tstatus code: 400, request id: <redacted>, host id: <redacted>"}
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:266
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227
2022-07-27T20:52:34.164Z        DEBUG   exporter.field-export-reconciler        error did not need requeue      {"error": "the source resource is not synced yet"}

K8s bucket status:

Status:
  Ack Resource Metadata:
    Owner Account ID:  <redacted>
    Region:            us-gov-west-1
  Conditions:
    Message:               UnsupportedArgument: The request contained an unsupported argument.
                           status code: 400, request id: <redacted>, host id: <redacted>
    Status:                True
    Type:                  ACK.Recoverable
    Last Transition Time:  2022-07-28T00:35:13Z
    Message:               Unable to determine if desired resource state matches latest observed state
    Reason:                UnsupportedArgument: The request contained an unsupported argument.
                           status code: 400, request id: <redacted>, host id: <redacted>
    Status:                Unknown
    Type:                  ACK.ResourceSynced
  Location:                http://<redacted>.s3-us-gov-west-1.amazonaws.com/

The bucket does get created, but deletion of the bucket, or any additional spec configurations that are added, do not actually get applied to the resource. To be noted, we are working on implementing S3 ACK in us-gov-west-1 via an AWS GovCloud account.

Steps to reproduce

1. Define following values in Helm values.yaml

aws:
  region: us-gov-west-1

serviceAccount:
  # Specifies whether a service account should be created
  create: true
  # The name of the service account to use.
  name: ack-s3-controller
  annotations:
    eks.amazonaws.com/role-arn: "arn:aws-us-gov:iam::<redacted>:role/project/project-ldx/<redacted>"

log:
  level: debug
  enable_development_logging: true

2. Helm install

export SERVICE=s3
export RELEASE_VERSION=$(curl -sL https://api.github.com/repos/aws-controllers-k8s/$SERVICE-controller/releases/latest | grep '"tag_name":' | cut -d'"' -f4)
export ACK_SYSTEM_NAMESPACE=ack-system
helm upgrade --install -n $ACK_SYSTEM_NAMESPACE ack-$SERVICE-controller \
  oci://public.ecr.aws/aws-controllers-k8s/$SERVICE-chart --version="$RELEASE_VERSION" -f values.yaml

3. Apply bucket

cat <<EOF | kubectl apply -f -
apiVersion: s3.services.k8s.aws/v1alpha1
kind: Bucket
metadata:
  name: test-bucket
spec:
  name: test-bucket
EOF

Expected outcome An S3 bucket is created, no reconcile errors are shown in logs. Bucket can be updated, additional values can be specified in spec and applied correctly.

Initial intuition is the potential gap of features available in commercial AWS versus GovCloud AWS.

Environment

• Kubernetes version - 1.22
• Using EKS (yes/no), if so version? Yes, 1.22
• AWS service targeted (S3, RDS, etc.) S3

[RedbackThomson]

Initial intuition is the potential gap of features available in commercial AWS versus GovCloud AWS.

Yeah unfortunately I don't know the S3 service well enough to comment on what could be missing/extra here. All ACK controllers are built on the latest version of the models in the aws-sdk-go repository. That means any regions that do not support the latest version of those models may raise exceptions like this one. Without releasing separate versions for every region, I don't really know if there is a way around this.

[maddy-jo]

A follow-up on this issue (I'm a teammate of @stanenbaum-va): we forked the s3-controller repo and did some debugging on it. It appears that the call to GetBucketAccelerateConfigurationWithContext in addPutFieldsToSpec was the SDK call that caused the error.

We actually noticed that a very similar error is already handled by the controller when GetBucketAccelerateConfiguration is not supported in the region. The only difference is that, in our case, the error code returned by awsErr.Code() is UnsupportedArgument instead of MethodNotAllowed.

We're open to approaching this issue as potential contributors and submitting a PR, if the maintainers think that there's a fix that fits with the project. We're hoping that, since the controller is already handling a very similar error, there is a potential solution where that step can check both error codes and default to the empty configuration. Can you point us toward where this code is generated, to help us understand the context from the ACK side better?

In any case, we're going to continue debugging and looking into how this is generated. We plan to look into why there's this discrepancy in errors where GetBucketAccelerateConfiguration is not supported in the region (UnsupportedArgument vs MethodNotAllowed) and try to see if we can glean any more information about why we're seeing that error at the AWS API or aws-sdk-go levels. We'll also look independently into the ACK contributor docs and see if we can understand better how this code is generated.

[a-hilaly]

Hey @mjlumetta ,i'm not very familiar with s3 API, but my first assumption is that it is possible to implement this feature as you described. Let me investigate the feasibility of this feature and get back to you as soon as I have more confidence in this.

[maddy-jo]

Thanks @A-Hilaly! Much appreciated. For some additional context, here are a few other things we found.

• First, UnsupportedArgument is not listed as an error code in the S3 Error Responses documentation. This was a bit confusing to us, since we are getting that error code (unless it is intended to be documented elsewhere, for whatever reason).
• On the other hand, we found a handful of Github issues for other open source projects citing the same issue from users trying to run the tool in a GovCloud region. Here are examples in repos for hashicorp/terraform-provider-aws, crossplane-contrib/provider-aws, and iterate-ch/cyberduck. These issues seem to suggest that the UnsupportedArgument error code is returned for the GetBucketAccelerateConfiguration action in GovCloud generally, rather than something specific to our account or deployment.
• The GovCloud S3 docs do specify that Transfer Acceleration is not supported; however, they don't offer any information about API responses or error codes.

Hope this information is helpful context, and thanks again for your investigation into the issue.

[a-hilaly]

Hi @mjlumetta, sorry for the delay and thank you for providing all this information above.

I just took sometime to carefully go through the code and this issue. And i don't see any problem with the solution you are proposing here. I believe it is correct and definitely wouldn't hurt to have an extra check for UnsupportedArgument in https://github.com/aws-controllers-k8s/s3-controller/blob/v0.1.4/pkg/resource/bucket/hook.go#L302.

Would you be interested in contributing to the s3-controller? Happy to review et help get a new release containing the fix. Also i'm available on Kubernetes slack if you wanna chat about ACK tooling and code-generation system.

---

First, UnsupportedArgument is not listed as an error code in the S3 Error Responses documentation. This was a bit confusing to us, since we are getting that error code (unless it is intended to be documented elsewhere, for whatever reason).

I agree with you, i believe this kind of information should be documented. I'll reach out to our s3 point of contact to see what can we do about this.

[maddy-jo]

Thanks for this response @A-Hilaly – that's great news. My team and I would definitely be interested in contributing back to the s3-controller. We'll take a look at it shortly and can circle back with any new questions here or in the Kubernetes slack. Appreciate your follow-up and feedback.

[maddy-jo]

Hi @A-Hilaly, we opened up a PR in the s3-controller repo. It's just adding that additional condition to the if statement, so very small in terms of changes, but please let us know if there's anything else we need to do in terms of process, testing, stylistically, etc. Thank you!

[a-hilaly]

Thanks a lot @mjlumetta - just /lgtm-ed the PR will send a release PR very soon. For now i don't think this is possible to test using our prow infra :)