Invalid type for parameter ExternalId, value: None

[abaspinar]

Hi,

When I specify a role on sagify cloud train, it fails if I do not specify the ExternalId and got the following error.

Invalid type for parameter ExternalId, value: None, type: <class 'NoneType'>, valid types: <class 'str'>

In boto3 sts doc, it is defined asExternalId='string' so we can not pass None.

Can we assume role here without ExternalId if it is not specified from the cli arguments.

Using Sagify 0.20.4

[ilazakis]

Hi @abaspinar 👋, users should be able to provide or omit the external id according to their needs. I’ll flag this as a bug and fix asap.

[ilazakis]

Offending commit a366da6426abf873ce267cd339df6267add5660d

[ilazakis]

Using an empty string default value for external id here should do the trick.

[abaspinar]

Hi @ilazakis, I just tested and empty string is not working with the following error.

Invalid length for parameter ExternalId, value: 0, valid range: 2-inf

It seems that it needs at least 2 characters. I wonder what is the default value for that in botocore side.

[ilazakis]

Couldn't find the Python source, but Go also allows for a minimum of 2 characters for ExternalID, but only if it is not nil.

if s.ExternalId != nil && len(*s.ExternalId) < 2 {
    invalidParams.Add(aws.NewErrParamMinLen("ExternalId", 2))
}

I'm pretty sure the nil/None approach has worked for me but haven't used it in a while since I do most training using the EC2 instance's profile (which has a role with SageMaker access attached to it) without passing a role explicitly. In fact, I've been using a CD4ML solution we will be open-sourcing soon.

Will run a few tests over the weekend but we can always do the following:

if external_id is not None:
    assumedRoleObject = sts_client.assume_role(
                    RoleArn=aws_role,
                    RoleSessionName="SagifySession",
                    ExternalId=external_id)
else:
    assumedRoleObject = sts_client.assume_role(
                    RoleArn=aws_role,
                    RoleSessionName="SagifySession")

But it's still weird that these two (the else part of the conditional and using ExternalId=None in the if part) would be interpreted differently 🤷‍♂ Although I think in Python that's actually expected, so it should work. Then again, not passing anything would fail if no default value is provided and I can't see why the SDK would be using anything else than None for the default value.

[ilazakis]

I'll push the above and we can take it from there.

[ilazakis]

115 is now merged and available in 0.20.6 @abaspinar Please let me know if the issue persists and I will reopen. Thanks again for reporting!

[abaspinar]

Easy and quick fix. Thanks a lot. Can be changed later with the real default value of the botocore ExternalId but I also couldn't really find it when I was digging deeper the python sdk, maybe it is under the hood of somethings I am missing. Anyways, really appreciated for the fix.