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

fix: drop duplicate aws auth field in postgresql #3525

Merged
merged 3 commits into from
Sep 10, 2024

Conversation

Eileen-Yu
Copy link
Contributor

@Eileen-Yu Eileen-Yu commented Aug 30, 2024

Description

If choose postgresql AWS: Access Key ID and Secret Access Key as the Authentication Profile, there are multiple fields for AWS AccessKey, AWS Region, and AWS Secret Key" .

The reason is bc for PostgreSQL itself has awsIAM as the metadata, which already include accessKey, secretKey and awsRegion. But during the building it will append built-in auth profile again, which bring these duplicated fields. This PR tweak the logic of the building process, which filters out those fields for the postgres component.

Before the change:

    {
          "title": "AWS: Access Key ID and Secret Access Key",
          "description": "Authenticate using an Access Key ID and Secret Access Key included in the metadata",
          "metadata": [
            {
              "name": "useAWSIAM",
              "description": "Must be set to `true` to enable the component to retrieve access tokens from AWS IAM.\nThis authentication method only works with AWS Relational Database Service for PostgreSQL databases.",
              "required": true,
              "type": "bool",
              "example": "\"true\""
            },
            {
              "name": "connectionString",
              "description": "The connection string for the PostgreSQL database\nThis must contain the user, which corresponds to the name of the user created inside PostgreSQL that maps to the AWS IAM policy. This connection string should not contain any password. Note that the database name field is denoted by dbname with AWS.",
              "required": true,
              "sensitive": true,
              "type": "string",
              "example": "\"host=mydb.postgres.database.aws.com user=myapplication port=5432 dbname=dapr_test sslmode=require\"\n"
            },
            {
              "name": "awsRegion",
              "description": "The AWS Region where the AWS Relational Database Service is deployed to.",
              "required": true,
              "type": "string",
              "example": "\"us-east-1\""
            },
            {
              "name": "awsAccessKey",
              "description": "AWS access key associated with an IAM account.",
              "required": true,
              "type": "string",
              "example": "\"AKIAIOSFODNN7EXAMPLE\""
            },
            {
              "name": "awsSecretKey",
              "description": "The secret key associated with the access key.",
              "required": true,
              "sensitive": true,
              "type": "string",
              "example": "\"wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY\""
            },
            {
              "name": "awsRegion",
              "description": "The AWS Region where the AWS resource is deployed to.",
              "required": true,
              "type": "string",
              "example": "\"us-east-1\""
            },
            {
              "name": "accessKey",
              "description": "AWS access key associated with an IAM account",
              "required": true,
              "sensitive": true,
              "example": "\"AKIAIOSFODNN7EXAMPLE\""
            },
            {
              "name": "secretKey",
              "description": "The secret key associated with the access key",
              "required": true,
              "sensitive": true,
              "example": "\"wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY\""
            },
            {
              "name": "sessionToken",
              "description": "AWS session token to use. A session token is only required if you are using\ntemporary security credentials.",
              "sensitive": true,
              "type": "string",
              "example": "\"TOKEN\""
            }
          ]
        }

After the change:

        {
          "title": "AWS: Access Key ID and Secret Access Key",
          "description": "Authenticate using an Access Key ID and Secret Access Key included in the metadata",
          "metadata": [
            {
              "name": "useAWSIAM",
              "description": "Must be set to `true` to enable the component to retrieve access tokens from AWS IAM.\nThis authentication method only works with AWS Relational Database Service for PostgreSQL databases.",
              "required": true,
              "type": "bool",
              "example": "\"true\""
            },
            {
              "name": "connectionString",
              "description": "The connection string for the PostgreSQL database\nThis must contain the user, which corresponds to the name of the user created inside PostgreSQL that maps to the AWS IAM policy. This connection string should not contain any password. Note that the database name field is denoted by dbname with AWS.",
              "required": true,
              "sensitive": true,
              "type": "string",
              "example": "\"host=mydb.postgres.database.aws.com user=myapplication port=5432 dbname=dapr_test sslmode=require\"\n"
            },
            {
              "name": "awsRegion",
              "description": "The AWS Region where the AWS Relational Database Service is deployed to.",
              "required": true,
              "type": "string",
              "example": "\"us-east-1\""
            },
            {
              "name": "awsAccessKey",
              "description": "AWS access key associated with an IAM account.",
              "type": "string",
              "example": "\"AKIAIOSFODNN7EXAMPLE\""
            },
            {
              "name": "awsSecretKey",
              "description": "The secret key associated with the access key.",
              "sensitive": true,
              "type": "string",
              "example": "\"wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY\""
            },
            {
              "name": "sessionToken",
              "description": "AWS session token to use. A session token is only required if you are using\ntemporary security credentials.",
              "sensitive": true,
              "type": "string",
              "example": "\"TOKEN\""
            }
          ]
        },


This aligns with aws auth of other components.

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #[issue number]

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]

Copy link
Contributor

@cicoyle cicoyle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes lgtm, we definitely don't want duplicate fields. thx @Eileen-Yu.

cc @berndverst & @ItalyPaleAle & @yaron2 for confirmation & further approval

@Eileen-Yu Eileen-Yu force-pushed the fix/duplicate-field-in-postgresql branch from 7a432d2 to fe6fad8 Compare September 4, 2024 19:59
@Eileen-Yu Eileen-Yu force-pushed the fix/duplicate-field-in-postgresql branch from 6247eeb to 75ec5d5 Compare September 5, 2024 19:51
Copy link
Member

@artursouza artursouza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nit in comment.

@Eileen-Yu Eileen-Yu force-pushed the fix/duplicate-field-in-postgresql branch from 75ec5d5 to 9627bca Compare September 6, 2024 00:47
@yaron2 yaron2 added this pull request to the merge queue Sep 9, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 9, 2024
@Eileen-Yu Eileen-Yu closed this Sep 9, 2024
@Eileen-Yu Eileen-Yu reopened this Sep 9, 2024
@Eileen-Yu
Copy link
Contributor Author

Eileen-Yu commented Sep 9, 2024

golang ci lint seems failed for the code on main branch, will this block the merge? (sry I closed it accidentally 🤦‍♀️)

@sicoyle
Copy link
Contributor

sicoyle commented Sep 9, 2024

bump for review/merge pls. Linter failures are unrelated to these changes, and occurring afterrrr a merge capturing changes from main. Not sure how they got into main without being corrected first... cc @ItalyPaleAle

@yaron2 yaron2 merged commit dab1faa into dapr:main Sep 10, 2024
263 of 265 checks passed
elena-kolevska pushed a commit to elena-kolevska/components-contrib that referenced this pull request Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants