aws: Adding sts endpoint for CW and ES plugins #2501

MeghnaPrabhu · 2020-08-25T20:49:48Z

Added STS endpoint support for CW and ES plugin.
Add sts_endpoint option in eks_provider and aws_sts_provider
Added support to specify endpoint and sts_endpoint with or without the protocol.

E.g. endpoint logs.us-west-2.amazonaws.com and endpoint https://logs.us-west-2.amazonaws.com will work.

[OUTPUT]
    Name cloudwatch_logs
    Match *
    log_stream_prefix fluent-bit-
    log_group_name fluent
    region us-west-2
    auto_create_group On
    role_arn arn:aws:iam::xxxxxxxxxxxx:role/test
    sts_endpoint https://sts.us-west-2.amazonaws.com
    endpoint logs.us-west-2.amazonaws.com

[OUTPUT]
    Name  es
    Match *
    Host vpc-test-domain-ke7thhzoo7jawrhmz6mb7ite7y.us-west-2.es.amazonaws.com
    Port  443
    Index my_index
    Type  my_type
    Aws_Auth On
    Aws_Region us-west-2
    aws_role_arn arn:aws:iam::xxxxxxxxxxxx:role/test
    aws_sts_endpoint https://sts.us-west-2.amazonaws.com
    tls On
    tls.verify On
    tls.debug 1

Debug Logs

Valgrind Logs
valgrind_aws_credentials.log
valgrind_aws_credentials_sts.log

Signed-off-by: Meghna Prabhu meghnapr@amazon.com

Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

Example configuration file for the change
Debug log output from testing the change

Attached Valgrind output that shows no leaks or memory corruption was found

Documentation

Documentation required for this feature

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

PettitWesley · 2020-08-25T21:09:03Z

plugins/out_es/es.c

+    {
+     FLB_CONFIG_MAP_STR, "aws_sts_endpoint", "",
+     0, FLB_TRUE, offsetof(struct flb_elasticsearch, aws_sts_endpoint),
+     "STS endpoint"


Description should be longer. Like: "Custom endpoint for the AWS STS API, used with the AWS_Role_ARN option"

Also, since STS endpoint only makes sense if aws_role_arn is also specified, add a check for that. If aws_sts_endpoint is present without role_arn, print an error message

PettitWesley · 2020-08-25T21:09:30Z

plugins/out_cloudwatch_logs/cloudwatch_logs.c

+    {
+     FLB_CONFIG_MAP_STR, "sts_endpoint", NULL,
+     0, FLB_FALSE, 0,
+     "Specify a custom sts endpoint for the CloudWatch Logs API"


"Specify a custom endpoint for the STS API, can be used with the role_arn parameter"

Also, since STS endpoint only makes sense if role_arn is also specified, add a check for that. If sts_endpoint is present without role_arn, print and error message

PettitWesley · 2020-08-26T02:02:49Z

plugins/out_cloudwatch_logs/cloudwatch_logs.c

+    }
+
+    if (ctx->sts_endpoint && !ctx->role_arn) {
+        flb_plg_error(ctx->ins, " 'aws_role_arn' is a required option to use "


In the CloudWatch plugin its just called role_arn

Also see my other comment, I just realized this error isn't true or needed since we use sts_endpoint in the EKS provider

PettitWesley · 2020-08-26T02:12:04Z

plugins/out_es/es_conf.c

+            if (ctx->aws_sts_endpoint && !tmp) {
+                flb_plg_error(ctx->ins, " 'aws_role_arn' is a required option to use "


Sorry... I just realized... since we use sts_endpoint in the EKS Provider, this error message is no longer true. Role_arn is not required.

Does this mean that role_arn is not required even for the sts provider? I don't fully understand why the error message is not required. Can you explain.

Basically there are two cases:

role_arn is present, in which case we use the STS Provider which needs both the sts endpoint and the role as arguments

role_arn is not present, in which case we use the standard chain provider. That can contain the EKS provider, so it needs the sts endpoint as an argument

Originally, I had forgotten about case #2, so I thought #1 was the only case.

In both cases, the custom sts endpoint can be used

ooh! This makes sense! Thank you

PettitWesley · 2020-08-26T02:16:08Z

src/aws/flb_aws_credentials_sts.c

@@ -568,8 +576,11 @@ struct flb_aws_provider *flb_eks_provider_create(struct flb_config *config,
        return NULL;
    }

-    implementation->endpoint = flb_aws_endpoint("sts", region);
-    if (!implementation->endpoint) {
+     implementation->endpoint = removeProtocol(sts_endpoint, "https://");


Move this line inside of the if block below; strstr behavior is undefined if any of the parameters are NULL, so this code might work for you on your compiler, but it might not work on others: https://stackoverflow.com/questions/19579574/what-is-behavior-of-null-parameters-to-strstr

PettitWesley · 2020-08-26T02:16:29Z

src/aws/flb_aws_credentials_sts.c

+    if (implementation->endpoint) {
+        implementation->custom_endpoint = FLB_TRUE;
+    }else{
+        implementation->custom_endpoint = FLB_TRUE; 


I think you should set it to false in this case?

PettitWesley · 2020-08-26T02:17:48Z

src/aws/flb_aws_util.c

+char *removeProtocol (char *endpoint, char *protocol) {
+    if (strstr(endpoint, protocol)){
+        endpoint = endpoint + strlen(protocol);
+    }
+    return endpoint;
+}
+


strstr checks substring... we want to check prefix.

See here: https://stackoverflow.com/questions/4770985/how-to-check-if-a-string-starts-with-another-string-in-c

I like the second answer using strncmp

Oh yes! I see how using strstr is more error prone here. I will switch to strncmp.

PettitWesley · 2020-09-02T06:34:41Z

src/aws/flb_aws_credentials_sts.c

+    else {
+        implementation->custom_endpoint = FLB_FALSE; 
        goto error;
    }


Am I reading this code wrong... because it looks like if there is no custom endpoint, then we goto error and fail initialization...

PettitWesley · 2020-09-02T06:35:41Z

src/aws/flb_aws_credentials_sts.c

+    else {
+        implementation->custom_endpoint = FLB_FALSE; 
        goto error;
    }


Same question... looks like it will goto error if there is no custom endpoint

edsiper · 2020-09-25T22:39:58Z

@MeghnaPrabhu thanks for this PR. Please resolve the conflicts on src/aws/flb_aws_util.c

Signed-off-by: Meghna Prabhu <meghnapr@amazon.com>

Signed-off-by: Wesley Pettit <wppttt@amazon.com>

PettitWesley · 2020-09-27T08:13:57Z

@meghnapr @edsiper Fixed the conflicts and added sts_endpoint to new plugins out_s3 and out_kinesis_firehose

MeghnaPrabhu requested review from edsiper, fujimotos, koleini and PettitWesley as code owners August 25, 2020 20:49

PettitWesley reviewed Aug 25, 2020

View reviewed changes

MeghnaPrabhu force-pushed the aws_sts_endpoint branch 3 times, most recently from 6378413 to 3636223 Compare August 26, 2020 01:18

PettitWesley reviewed Aug 26, 2020

View reviewed changes

MeghnaPrabhu force-pushed the aws_sts_endpoint branch from 3636223 to f3d34e9 Compare August 26, 2020 17:45

PettitWesley reviewed Sep 2, 2020

View reviewed changes

MeghnaPrabhu force-pushed the aws_sts_endpoint branch from f3d34e9 to 513982a Compare September 2, 2020 16:59

edsiper added the waiting-for-user Waiting for more information, tests or requested changes label Sep 25, 2020

meghnapr and others added 7 commits September 27, 2020 00:35

out_cloudwatch_logs: add support for custom sts endpoint

935a1df

Signed-off-by: Meghna Prabhu <meghnapr@amazon.com>

out_es: add support for custom STS endpoint

daa62f6

Signed-off-by: Meghna Prabhu <meghnapr@amazon.com>

aws: add support for custom STS endpoint in flb_aws_sts_provider

eef060b

Signed-off-by: Meghna Prabhu <meghnapr@amazon.com>

aws: add support for custom sts endpoint in flb_eks_provider

363771c

Signed-off-by: Meghna Prabhu <meghnapr@amazon.com>

aws: sts: set custom_endpoint flag correctly

39d4d4c

Signed-off-by: Wesley Pettit <wppttt@amazon.com>

out_s3: Add sts_endpoint and role_arn options

76685df

Signed-off-by: Wesley Pettit <wppttt@amazon.com>

out_kinesis_firehose: Add sts_endpoint option

677a155

Signed-off-by: Wesley Pettit <wppttt@amazon.com>

PettitWesley force-pushed the aws_sts_endpoint branch from 513982a to 677a155 Compare September 27, 2020 08:12

PettitWesley approved these changes Sep 27, 2020

View reviewed changes

PettitWesley merged commit 8d412ed into fluent:master Sep 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

aws: Adding sts endpoint for CW and ES plugins #2501

aws: Adding sts endpoint for CW and ES plugins #2501

MeghnaPrabhu commented Aug 25, 2020 •

edited

Loading

PettitWesley Aug 25, 2020

PettitWesley Aug 25, 2020 •

edited

Loading

PettitWesley Aug 25, 2020

PettitWesley Aug 25, 2020

PettitWesley Aug 26, 2020

PettitWesley Aug 26, 2020

PettitWesley Aug 26, 2020

MeghnaPrabhu Aug 26, 2020

PettitWesley Aug 26, 2020 •

edited

Loading

MeghnaPrabhu Aug 26, 2020

PettitWesley Aug 26, 2020

PettitWesley Aug 26, 2020

PettitWesley Aug 26, 2020

MeghnaPrabhu Aug 26, 2020

PettitWesley Sep 2, 2020

PettitWesley Sep 2, 2020

edsiper commented Sep 25, 2020

PettitWesley commented Sep 27, 2020

		if (ctx->aws_sts_endpoint && !tmp) {
		flb_plg_error(ctx->ins, " 'aws_role_arn' is a required option to use "

aws: Adding sts endpoint for CW and ES plugins #2501

aws: Adding sts endpoint for CW and ES plugins #2501

Conversation

MeghnaPrabhu commented Aug 25, 2020 • edited Loading

Choose a reason for hiding this comment

PettitWesley Aug 25, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PettitWesley Aug 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edsiper commented Sep 25, 2020

PettitWesley commented Sep 27, 2020

MeghnaPrabhu commented Aug 25, 2020 •

edited

Loading

PettitWesley Aug 25, 2020 •

edited

Loading

PettitWesley Aug 26, 2020 •

edited

Loading