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

Unable to fetch bundles: S3 returns 501 Not Implemented #5472

Closed
Hiieu opened this issue Dec 12, 2022 · 2 comments · Fixed by #5475
Closed

Unable to fetch bundles: S3 returns 501 Not Implemented #5472

Hiieu opened this issue Dec 12, 2022 · 2 comments · Fixed by #5475
Labels

Comments

@Hiieu
Copy link

Hiieu commented Dec 12, 2022

Hi there 👋,
I wonder if anyone has a similar issue after upgrading OPA (running on ECS) to v0.47.0:
the application no longer can fetch bundles from S3.
The S3 service sends back an HTTP 501 code: 501 Not Implemented

Short description

After upgrading OPA from v0.46.0 to v0.47.0 and without changing any configurations, the application cannot fetch bundles from the S3 bucket.

We haven't noticed any changes in the GET requests to S3, but for some reason, this error is only returned in v0.47.X. After downgrading the version to 0.46.X OPA is able to fetch bundles without any issues.

Version: v0.47.0
Where OPA is deployed: AWS ECS

OPA config file:

services:
  s3_discovery_bundle:
    url: https://<REDACTED>.s3.eu-west-1.amazonaws.com
    credentials:
      s3_signing:
        metadata_credentials:
          aws_region: eu-west-1
  s3_bundles:
    url: https://<REDACTED>.s3.eu-west-1.amazonaws.com
    credentials:
      s3_signing:
        metadata_credentials:
          aws_region: eu-west-1
discovery:
  service: s3_discovery_bundle
  resource: discovery.tar.gz
  decision: discovery/config
  polling:
    long_polling_timeout_seconds: 45
  signing:
    keyid: discovery_key
keys:
  discovery_key:
    key: <This value is populated by "opa run --set-file">

Command:

["run","--server","--log-level=debug","--log-format=json","--disable-telemetry","--config-file=/tmp/config.yaml","--set-file=keys.discovery_key.key=/tmp/discovery_key"]

Steps To Reproduce

  1. Deploy OPA v0.47.X on ECS and use S3 signing.
  2. The S3 service should return HTTP 501.

Expected behavior

OPA should be able to fetch bundles from S3 buckets without any issues.

Additional context

Logs from Cloudwatch:

Timestamp | Message | Container
-- | -- | --
06/12/2022, 11:45:31 | {
    "headers": {
        "Content-Type": [
            "application/xml"
        ],
        "Date": [
            "Tue, 06 Dec 2022 11:45:31 GMT"
        ],
        "Server": [
            "AmazonS3"
        ],
        "X-Amz-Id-2": [
            "DsXSPn9+KByKKLUyvwIp9uToGYX2z3z29EzMuCOHr8bVLyvp5I1xCzMwzTM3Ya6zwIs1Hkc5Fc8="
        ],
        "X-Amz-Request-Id": [
            "DXTYQZHBH33R6DQJ"
        ]
    },
    "level": "debug",
    "method": "GET",
    "msg": "Received response.",
    "status": "501 Not Implemented",
    "time": "2022-12-06T11:45:31Z",
    "url": "https://<REDACTED>.s3.eu-west-1.amazonaws.com/discovery.tar.gz"
}
 | opa
06/12/2022, 11:45:31 | {
    "level": "error",
    "msg": "Discovery download failed: server replied with Not Implemented",
    "plugin": "discovery",
    "time": "2022-12-06T11:45:31Z"
}
 | opa
06/12/2022, 11:45:31 | {
    "level": "debug",
    "msg": "Waiting 2m4.462710453s before next download/retry.",
    "time": "2022-12-06T11:45:31Z"
}
 | opa
06/12/2022, 11:45:31 | {
    "level": "debug",
    "msg": "Download starting.",
    "time": "2022-12-06T11:45:31Z"
}
 | opa
06/12/2022, 11:45:31 | {
    "level": "debug",
    "msg": "Signing request with AWS credentials.",
    "time": "2022-12-06T11:45:31Z"
}
 | opa
06/12/2022, 11:45:31 | {
    "level": "debug",
    "msg": "Credentials previously obtained from metadata service still valid.",
    "time": "2022-12-06T11:45:31Z"
}
 | opa
06/12/2022, 11:45:31 | {
    "level": "debug",
    "msg": "awsSigningAuthPlugin:*rest.awsMetadataCredentialService successful",
    "time": "2022-12-06T11:45:31Z"
}
 | opa
06/12/2022, 11:45:31 | {
    "headers": {
        "Authorization": [
            "REDACTED"
        ],
        "Host": [
            "<REDACTED>.s3.eu-west-1.amazonaws.com"
        ],
        "Prefer": [
            "modes=snapshot,delta;wait=45"
        ],
        "User-Agent": [
            "Open Policy Agent/0.47.0 (linux, amd64)"
        ],
        "X-Amz-Content-Sha256": [
            "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
        ],
        "X-Amz-Date": [
            "20221206T114531Z"
        ],
        "X-Amz-Security-Token": [
            "<REDACTED>"
        ]
    },
    "level": "debug",
    "method": "GET",
    "msg": "Sending request.",
    "time": "2022-12-06T11:45:31Z",
    "url": "https://<REDACTED>.s3.eu-west-1.amazonaws.com/discovery.tar.gz"
}

@Hiieu Hiieu added the bug label Dec 12, 2022
@anderseknert
Copy link
Member

Hi @Hiieu 👋 And thanks for reporting this! I wonder if it could be related to the work that went into AWS v4 signing for the http.send built-in function. While they're obviously different things, I think some work was made to have them share code between them. Either way, we'll look into it.

@srenatus
Copy link
Contributor

A header that you provided implies functionality that is not implemented.

https://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html#ErrorCodeList

So maybe comparing headers with before/after is a way to start here

philipaconrad added a commit to philipaconrad/opa that referenced this issue Dec 12, 2022
This commit refactors the shared AWS Sig v4 signing code, specifically
to prevent the issue behind open-policy-agent#5472. The underlying problem for open-policy-agent#5472 was
that the `"Authorization"` header was being appended *twice* to the
request, but only for the AWS REST plugin, because the value was pulled
twice from the signed headers map.

This was not caught by the unit tests, because the REST plugin's unit
tests all assumed the header was single-valued and canonicalized.

We now explicitly test for that condition in the unit tests, and the
signing code now returns the AWS headers map separately from the value
for the `"Authorization"` header, reducing the potential for this
mistake to happen in the future.

Fixes: open-policy-agent#5472

Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
philipaconrad added a commit that referenced this issue Dec 12, 2022
…5475)

This commit refactors the shared AWS Sig v4 signing code, specifically
to prevent the issue behind #5472. The underlying problem for was
that the `"Authorization"` header was being appended *twice* to the
request, but only for the AWS REST plugin, because the value was pulled
twice from the signed headers map.

This was not caught by the unit tests, because the REST plugin's unit
tests all assumed the header was single-valued and canonicalized.

We now explicitly test for that condition in the unit tests, and the
signing code now returns the AWS headers map separately from the value
for the `"Authorization"` header, reducing the potential for this
mistake to happen in the future.

Fixes: #5472

Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
philipaconrad added a commit to philipaconrad/opa that referenced this issue Dec 12, 2022
…pen-policy-agent#5475)

This commit refactors the shared AWS Sig v4 signing code, specifically
to prevent the issue behind open-policy-agent#5472. The underlying problem for was
that the `"Authorization"` header was being appended *twice* to the
request, but only for the AWS REST plugin, because the value was pulled
twice from the signed headers map.

This was not caught by the unit tests, because the REST plugin's unit
tests all assumed the header was single-valued and canonicalized.

We now explicitly test for that condition in the unit tests, and the
signing code now returns the AWS headers map separately from the value
for the `"Authorization"` header, reducing the potential for this
mistake to happen in the future.

Fixes: open-policy-agent#5472

Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
(cherry picked from commit 1d1cb35)
philipaconrad added a commit that referenced this issue Dec 12, 2022
…5475)

This commit refactors the shared AWS Sig v4 signing code, specifically
to prevent the issue behind #5472. The underlying problem for was
that the `"Authorization"` header was being appended *twice* to the
request, but only for the AWS REST plugin, because the value was pulled
twice from the signed headers map.

This was not caught by the unit tests, because the REST plugin's unit
tests all assumed the header was single-valued and canonicalized.

We now explicitly test for that condition in the unit tests, and the
signing code now returns the AWS headers map separately from the value
for the `"Authorization"` header, reducing the potential for this
mistake to happen in the future.

Fixes: #5472

Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
(cherry picked from commit 1d1cb35)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants