AWS::SSM::Parameter resource #159

ferozbaig-amzn · 2022-02-01T09:39:06Z

Development of AWS::SSM::Parameter resource

aws-ssm-parameter/aws-ssm-parameter.json

aws-ssm-parameter/src/main/java/com/amazonaws/ssm/parameter/ClientBuilder.java

aws-ssm-parameter/aws-ssm-parameter.json

aws-ssm-parameter/src/main/java/com/amazonaws/ssm/parameter/CreateHandler.java

aws-ssm-parameter/src/main/java/com/amazonaws/ssm/parameter/BaseHandlerStd.java

praghu1 · 2022-02-03T05:54:00Z

aws-ssm-parameter/src/main/java/com/amazonaws/ssm/parameter/BaseHandlerStd.java

+			} else if (hasThrottled(e)) {
+				ex = new CfnThrottlingException(e);
+			} else {
+				ex = new CfnGeneralServiceException(e);


IllegalArgumentException ?

Can't find this Exception in javadoc for AWS::SSM::Parameter Resource. Unable to reproduce this error.

Pass in bad inputs and check what exceptions are thrown. It might map to InvalidRequest

Thanks. Added more exception handling cases.

aws-ssm-parameter/src/main/java/com/amazonaws/ssm/parameter/BaseHandlerStd.java

advaj

There seems to be a lot of deviation between native and this code base.

I have gone through everything except Update. But Create and Update are similar. Can you make the changes requested and when I look at it a second time, I will take a look at Update as well.

advaj · 2022-02-04T07:09:33Z

aws-ssm-parameter/aws-ssm-parameter.json

-            "description": "The information about the parameter.",
-            "minLength": 0,
-            "maxLength": 1024
+            "description": "Information about the parameter."
        },
        "Policies": {
            "type": "string",


This should be an object - it can take both a JSON and a String? Can you test this to verify? (Stringification is usually applicable to policies, but please verify it)

CFN Doc says Policies is of type String

Tried creating it via CLI with Json value instead of String, and faced below error.

Parameter validation failed:
Invalid type for parameter Policies

aws-ssm-parameter/src/main/java/com/amazonaws/ssm/parameter/CreateHandler.java

advaj · 2022-02-04T07:34:11Z

aws-ssm-parameter/src/main/java/com/amazonaws/ssm/parameter/CreateHandler.java

+		if (model.getName() == null) {
+			model.setName(IdentifierUtils.generateResourceIdentifier("CFN",
+				request.getLogicalResourceIdentifier(),
+				request.getClientRequestToken(), 40));


How did you arrive at 40? Also, you dont seem to be truncating the length of the logical resource id. WHy not just use the same function as native. Much easier. (Use the same values too)

Reverted back to native implementation

advaj · 2022-02-04T07:49:29Z

aws-ssm-parameter/src/main/java/com/amazonaws/ssm/parameter/CreateHandler.java

+				.makeServiceCall((putParameterRequest, ssmProxyClient) ->
+					ssmProxyClient.injectCredentialsAndInvokeV2(putParameterRequest, ssmProxyClient.client()::putParameter))


It seems like there are two workflows here:

When the SSM parameter is not of EC2 type - then we just create the SSM parameter.

If it is of EC2 type, then we use versioning.

Where have you handled this?

This has been handled

advaj · 2022-02-04T07:51:59Z

aws-ssm-parameter/src/main/java/com/amazonaws/ssm/parameter/CreateHandler.java

+				.makeServiceCall((putParameterRequest, ssmProxyClient) ->
+					ssmProxyClient.injectCredentialsAndInvokeV2(putParameterRequest, ssmProxyClient.client()::putParameter))
+				.stabilize((req, response, client, model1, cbContext) -> stabilize(req, response, client, model1, cbContext, logger))
+				.handleError((req, e, proxy1, model1, context1) -> handleError(req, e, proxy1, model1, context1, logger))


Also, it seems like AlreadyExistsExceptions are swallowed after comparing to check if the resource OOB has identical configuration.

This might cause contract test failures though - so we need to call this out. (Office hours item)

So we let it pass if the resource exists with identical configuration ? A confirmation would help.

advaj · 2022-02-04T08:56:05Z

aws-ssm-parameter/src/main/java/com/amazonaws/ssm/parameter/CreateHandler.java

+				.backoffDelay(getBackOffDelay(model))
+				.makeServiceCall((putParameterRequest, ssmProxyClient) ->
+					ssmProxyClient.injectCredentialsAndInvokeV2(putParameterRequest, ssmProxyClient.client()::putParameter))
+				.stabilize((req, response, client, model1, cbContext) -> stabilize(req, response, client, model1, cbContext, logger))


Stabilization is called only for EC2 data types. You seem to be stabilizing even when the type is not EC2?

advaj · 2022-02-04T08:56:40Z

aws-ssm-parameter/src/main/java/com/amazonaws/ssm/parameter/BaseHandlerStd.java

+				return false;
+			}
+			return (response.parameters() != null && response.parameters().get(0).version() == putParameterResponse.version());
+		} catch (Exception e) {


Handle Throttling here. If the exception type is of Throttling, then you can return false and retry.

advaj · 2022-02-04T08:58:38Z

aws-ssm-parameter/src/main/java/com/amazonaws/ssm/parameter/BaseHandlerStd.java

+		}
+	}
+
+	protected ProgressEvent<ResourceModel, CallbackContext> handleError(SsmRequest ssmRequest, Exception e, ProxyClient<SsmClient> proxyClient,


Native is handling a lot more errors - look for this -

` private static final Set THROTTLING_ERROR_CODES = ImmutableSet.of(
"ThrottlingException",
"TooManyUpdates");

private static final Set<String> ERROR_CODES_THAT_DEFAULT_TO_RETRIABLE_UNTOUCHED = Sets.union( Collections.singleton(HTTP_CONNECTION_TIMEOUT_ERROR_CODE), THROTTLING_ERROR_CODES); private static final Set<String> ERROR_CODES_THAT_DEFAULT_TO_RETRIABLE_TOUCHED = ImmutableSet.of( HTTP_TIMEOUT_ERROR_CODE, HTTP_FAILURE_ERROR_CODE);`

It's counterpart hasThrottled() has already been implemented which looks for errorCode.

advaj · 2022-02-04T09:01:54Z

aws-ssm-parameter/src/main/java/com/amazonaws/ssm/parameter/ReadHandler.java

+		logger.log("Invoking Read Handler");
+		logger.log("READ ResourceModel: " + model.toString());
+
+		return ProgressEvent.progress(model, callbackContext)


Can you verify to see if Drift if working for values other than Type/Value? Looks like native is only checking for these 2 attributes.

Drift does not work for Native. Yes Native only checks for 2 attributes. Current Uluru implementation supports all attributes.

advaj · 2022-02-04T09:02:20Z

aws-ssm-parameter/src/main/java/com/amazonaws/ssm/parameter/ReadHandler.java

+		return ProgressEvent.progress(model, callbackContext)
+			.then(progress -> getParameters(proxy, progress, proxyClient, model, callbackContext, logger))
+			.then(progress -> describeParameters(proxy, progress, proxyClient, progress.getResourceModel(), progress.getCallbackContext(), logger))
+			.then(progress -> listTagsForResourceRequestForParameters(proxy, progress, proxyClient, progress.getResourceModel(), progress.getCallbackContext(), logger))


You need to soft-fail wherever extra permissions are needed. (Like ListTags)

ferozbaig-amzn added 11 commits January 15, 2022 12:40

pom version changes

002661d

amend schema

41748ff

rearranged code spacing

95b2be4

rectified CREATE handler

d8e3298

rectified READ

c72321a

rectified DELETE

e7b6b24

rectified UPDATE

28c7f0f

rectified LIST

8fd2be4

added Client Side Throttling with Client re-use

418b547

added unit tests

8b05981

fixed pre-commit error

e588355

praghu1 reviewed Feb 3, 2022

View reviewed changes

ferozbaig-amzn added 5 commits February 3, 2022 14:18

fix: taggable true

950a0f9

fix: reduce retries to 4

26cb109

fix: add READ permissions in CREATE as READ is called from within CREATE

13c6e5d

fix: return error msg instead of wrapping in an exception

0d3f8e2

fix: name generation

c6afbbc

advaj suggested changes Feb 4, 2022

View reviewed changes

ferozbaig-amzn added 7 commits February 8, 2022 12:00

added more exception handling cases

a5d4e25

fix to avoid NPE

fbe04c0

generate param name as per native logic

fce48a8

stabilize only for aws::ec2 datatype

0ff512d

handle throttling in case of stabilize

4188600

soft-fail for extra api calls

49027ee

fix unit test

925a8c4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AWS::SSM::Parameter resource #159

AWS::SSM::Parameter resource #159

ferozbaig-amzn commented Feb 1, 2022

praghu1 Feb 3, 2022

ferozbaig-amzn Feb 3, 2022

advaj Feb 4, 2022

ferozbaig-amzn Feb 8, 2022

advaj left a comment

advaj Feb 4, 2022

ferozbaig-amzn Feb 8, 2022

ferozbaig-amzn Feb 9, 2022

advaj Feb 4, 2022

ferozbaig-amzn Feb 9, 2022

advaj Feb 4, 2022

ferozbaig-amzn Feb 9, 2022

advaj Feb 4, 2022

advaj Feb 4, 2022

ferozbaig-amzn Feb 8, 2022

advaj Feb 4, 2022

ferozbaig-amzn Feb 8, 2022

advaj Feb 4, 2022

ferozbaig-amzn Feb 9, 2022

advaj Feb 4, 2022

ferozbaig-amzn Feb 8, 2022 •

edited

Loading

advaj Feb 4, 2022

ferozbaig-amzn Feb 8, 2022

advaj Feb 4, 2022

ferozbaig-amzn Feb 9, 2022

		.makeServiceCall((putParameterRequest, ssmProxyClient) ->
		ssmProxyClient.injectCredentialsAndInvokeV2(putParameterRequest, ssmProxyClient.client()::putParameter))

AWS::SSM::Parameter resource #159

Are you sure you want to change the base?

AWS::SSM::Parameter resource #159

Conversation

ferozbaig-amzn commented Feb 1, 2022

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

advaj left a 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

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

Choose a reason for hiding this comment

ferozbaig-amzn Feb 8, 2022 • 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

ferozbaig-amzn Feb 8, 2022 •

edited

Loading