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.

Sign up for GitHub

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

feat: add External ID parameter to AWS converter #57

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

feat: add External ID parameter to AWS converter #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6422d1d
Add External ID parameter to AWS converter
artydeez Jan 25, 2022
fc53422
add aws iam role example
artydeez Jan 25, 2022
a007e07
add test for external ID
artydeez Jan 25, 2022
0c6890e
add missing external id example
artydeez Jan 25, 2022
9abbf7d
adding test yamls for external id
artydeez Jan 25, 2022
1d3ba2c
remove external id from missing ext id example
artydeez Feb 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions docs/examples/aws-credentials-iam-role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
apiVersion: v1
Copy link
Member

Choose a reason for hiding this comment

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

This isn't referenced from the examples page (or elsewhere?
Could you update the existing Aws example with some sommebts for optionality of the fields?

Copy link
Author

Choose a reason for hiding this comment

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

I don't understand. Do you mean that I should add a reference within the examples README?

Copy link
Member

Choose a reason for hiding this comment

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

Yes these docs/ will not be visible I documentation unless you reference them somewhere.
https://github.com/jenkinsci/kubernetes-credentials-provider-plugin/blob/master/docs/examples/README.md#aws-credentials

kind: Secret
metadata:
# this is the jenkins id.
name: "test-aws-credentials"
labels:
# so we know what type it is.
"jenkins.io/credentials-type": "aws"
annotations:
# description - can not be a label as spaces are not allowed
"jenkins.io/credentials-description" : "credentials from Kubernetes"
type: Opaque
stringData:
accessKey: ""
secretKey: ""
iamRoleArn: IamRoleArn
iamExternalId: IamExternalID
11 changes: 10 additions & 1 deletion ...s/jenkins/plugins/kubernetes_credentials_provider/convertors/AWSCredentialsConvertor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,13 @@ public AWSCredentialsImpl convert(Secret secret) throws CredentialsConvertionExc
if (iamMfaSerialNumberBase64.isPresent()) {
iamMfaSerialNumber = SecretUtils.requireNonNull(SecretUtils.base64DecodeToString(iamMfaSerialNumberBase64.get()), "aws credential has an invalid iamMfaSerialNumber (must be base64 encoded UTF-8)");
}

Optional<String> iamExternalIdBase64 = SecretUtils.getOptionalSecretData(secret, "iamExternalId", "aws credential: failed to retrieve optional parameter iamExternalId");
String iamExternalId = null;

if (iamExternalIdBase64.isPresent()) {
iamExternalId = SecretUtils.requireNonNull(SecretUtils.base64DecodeToString(iamExternalIdBase64.get()), "aws credential has an invalid iamExternalId (must be base64 encoded UTF-8)");
}

return new AWSCredentialsImpl(
// Scope
Expand All @@ -83,7 +90,9 @@ public AWSCredentialsImpl convert(Secret secret) throws CredentialsConvertionExc
// IAM Role ARN
iamRoleArn,
// MFA
iamMfaSerialNumber
iamMfaSerialNumber,
// External ID
iamExternalId
);

}
Expand Down
25 changes: 25 additions & 0 deletions ...nkins/plugins/kubernetes_credentials_provider/convertors/AWSCredentialsConvertorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public class AWSCredentialsConvertorTest {
String secretKey= "Pa$$word";
String iamRoleArn = "ecr:eu-west-1:86c8f5ec-1ce1-4e94-80c2-18e23bbd724a";
String iamMfaSerialNumber = "GAHT12345678";
String iamExternalId = "ext-id";

@Test
public void canConvert() throws Exception {
Expand All @@ -71,6 +72,7 @@ public void canConvertAValidSecret() throws Exception {
assertThat("credential secretKey is mapped correctly", credential.getSecretKey().getPlainText(), is(secretKey));
assertThat("credential iamRoleArn is mapped correctly", credential.getIamRoleArn(), is(iamRoleArn));
assertThat("credential iamMfaSerialNumber is mapped correctly", credential.getIamMfaSerialNumber(), is(iamMfaSerialNumber));
assertThat("credential iamExternalId is mapped correctly", credential.getIamExternalId(), is(iamExternalId));
}
}

Expand Down Expand Up @@ -107,6 +109,7 @@ public void canConvertAValidMappedSecret() throws Exception {
assertThat("credential secretKey is mapped correctly", credential.getSecretKey().getPlainText(), is(secretKey));
assertThat("credential iamRoleArn is mapped correctly", credential.getIamRoleArn(), is(iamRoleArn));
assertThat("credential iamMfaSerialNumber is mapped correctly", credential.getIamMfaSerialNumber(), is(iamMfaSerialNumber));
assertThat("credential iamExternalId is mapped correctly", credential.getIamExternalId(), is(iamExternalId));
}
}

Expand All @@ -126,6 +129,7 @@ public void canConvertAValidSecretWithNoDescription() throws Exception {
assertThat("credential secretKey is mapped correctly", credential.getSecretKey().getPlainText(), is(secretKey));
assertThat("credential iamRoleArn is mapped correctly", credential.getIamRoleArn(), is(iamRoleArn));
assertThat("credential iamMfaSerialNumber is mapped correctly", credential.getIamMfaSerialNumber(), is(iamMfaSerialNumber));
assertThat("credential iamExternalId is mapped correctly", credential.getIamExternalId(), is(iamExternalId));
}
}

Expand All @@ -144,6 +148,7 @@ public void canConvertAValidSecretWithNoIamRole() throws Exception {
assertThat("credential accessKey is mapped correctly", credential.getAccessKey(), is(accessKey));
assertThat("credential secretKey is mapped correctly", credential.getSecretKey().getPlainText(), is(secretKey));
assertThat("credential iamMfaSerialNumber is mapped correctly", credential.getIamMfaSerialNumber(), is(iamMfaSerialNumber));
assertThat("credential iamExternalId is mapped correctly", credential.getIamExternalId(), is(iamExternalId));
}
}

Expand All @@ -162,6 +167,25 @@ public void canConvertAValidSecretWithNoIamMfa() throws Exception {
assertThat("credential accessKey is mapped correctly", credential.getAccessKey(), is(accessKey));
assertThat("credential secretKey is mapped correctly", credential.getSecretKey().getPlainText(), is(secretKey));
assertThat("credential iamRoleArn is mapped correctly", credential.getIamRoleArn(), is(iamRoleArn));
assertThat("credential iamExternalId is mapped correctly", credential.getIamExternalId(), is(iamExternalId));
}
}

@Test
public void canConvertAValidSecretWithNoIamExternalId() throws Exception {
AWSCredentialsConvertor convertor = new AWSCredentialsConvertor();

try (InputStream is = get("validMissingIamExternalId.yaml")) {
Secret secret = Serialization.unmarshal(is, Secret.class);
assertThat("The Secret was loaded correctly from disk", notNullValue());
AWSCredentialsImpl credential = convertor.convert(secret);
assertThat(credential, notNullValue());
assertThat("credential id is mapped correctly", credential.getId(), is("a-test-aws"));
assertThat("credential description is mapped correctly", credential.getDescription(), is(emptyString()));
assertThat("credential scope is mapped correctly", credential.getScope(), is(CredentialsScope.GLOBAL));
assertThat("credential accessKey is mapped correctly", credential.getAccessKey(), is(accessKey));
assertThat("credential secretKey is mapped correctly", credential.getSecretKey().getPlainText(), is(secretKey));
assertThat("credential iamRoleArn is mapped correctly", credential.getIamRoleArn(), is(iamRoleArn));
Copy link
Member

Choose a reason for hiding this comment

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

Should you add a check that the external id check
is null here?

Copy link
Author

Choose a reason for hiding this comment

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

@jtnord can you elaborate or add an example? None of the other missing item validations are explicitely checking if that value is null

Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/jenkinsci/kubernetes-credentials-provider-plugin/blob/master/src/test/java/com/cloudbees/jenkins/plugins/kubernetes_credentials_provider/convertors/OpenstackCredentialv3ConvertorTest.java#L136

}
}

Expand All @@ -182,6 +206,7 @@ public void canConvertAValidScopedSecret() throws Exception {
assertThat("credential secretKey is mapped correctly", credential.getSecretKey().getPlainText(), is(secretKey));
assertThat("credential iamRoleArn is mapped correctly", credential.getIamRoleArn(), is(iamRoleArn));
assertThat("credential iamMfaSerialNumber is mapped correctly", credential.getIamMfaSerialNumber(), is(iamMfaSerialNumber));
assertThat("credential iamExternalId is mapped correctly", credential.getIamExternalId(), is(iamExternalId));
}
}

Expand Down
1 change: 1 addition & 0 deletions ...kubernetes_credentials_provider/convertors/AWSCredentialsConvertorTest/valid-no-desc.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ data:
secretKey: UGEkJHdvcmQ= #Pa$$word
iamRoleArn: ZWNyOmV1LXdlc3QtMTo4NmM4ZjVlYy0xY2UxLTRlOTQtODBjMi0xOGUyM2JiZDcyNGE= #ecr:eu-west-1:86c8f5ec-1ce1-4e94-80c2-18e23bbd724a
iamMfaSerialNumber: R0FIVDEyMzQ1Njc4 #GAHT12345678
iamExternalId: ZXh0LWlk #ext-id

1 change: 1 addition & 0 deletions ...plugins/kubernetes_credentials_provider/convertors/AWSCredentialsConvertorTest/valid.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ data:
secretKey: UGEkJHdvcmQ= #Pa$$word
iamRoleArn: ZWNyOmV1LXdlc3QtMTo4NmM4ZjVlYy0xY2UxLTRlOTQtODBjMi0xOGUyM2JiZDcyNGE= #ecr:eu-west-1:86c8f5ec-1ce1-4e94-80c2-18e23bbd724a
iamMfaSerialNumber: R0FIVDEyMzQ1Njc4 #GAHT12345678
iamExternalId: ZXh0LWlk #ext-id

5 changes: 4 additions & 1 deletion ...s/kubernetes_credentials_provider/convertors/AWSCredentialsConvertorTest/validMapped.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@ metadata:
"jenkins.io/credentials-keybinding-iamRoleArn" : "r"
# map the iamMfaSerialNumber field to m
"jenkins.io/credentials-keybinding-iamMfaSerialNumber" : "m"
# map the iamExternalId field to e
"jenkins.io/credentials-keybinding-iamExternalId" : "e"
type: Opaque
data:
# UTF-8 base64 encoded
a: QUJDMTIzNDU2 #ABC123456
s: UGEkJHdvcmQ= #Pa$$word
r: ZWNyOmV1LXdlc3QtMTo4NmM4ZjVlYy0xY2UxLTRlOTQtODBjMi0xOGUyM2JiZDcyNGE= #ecr:eu-west-1:86c8f5ec-1ce1-4e94-80c2-18e23bbd724a
m: R0FIVDEyMzQ1Njc4 #GAHT12345678
m: R0FIVDEyMzQ1Njc4 #GAHT12345678
e: ZXh0LWlk #ext-id
26 changes: 26 additions & 0 deletions ...redentials_provider/convertors/AWSCredentialsConvertorTest/validMissingIamExternalId.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
apiVersion: v1
kind: Secret
metadata:
# this is the jenkins id.
name: "a-test-aws"
labels:
# so we know what type it is.
"jenkins.io/credentials-type": "aws"
annotations:
# description - can not be a label as spaces are not allowed
"jenkins.io/credentials-description" : "credentials from Kubernetes"
# map the accessKey field to a
"jenkins.io/credentials-keybinding-accessKey" : "a"
# map the secretKey field to s
"jenkins.io/credentials-keybinding-secretKey" : "s"
# map the iamRoleArn field to r
"jenkins.io/credentials-keybinding-iamRoleArn" : "r"
# map the iamMfaSerialNumber field to m
"jenkins.io/credentials-keybinding-iamMfaSerialNumber" : "m"
type: Opaque
data:
# UTF-8 base64 encoded
a: QUJDMTIzNDU2 #ABC123456
s: UGEkJHdvcmQ= #Pa$$word
r: ZWNyOmV1LXdlc3QtMTo4NmM4ZjVlYy0xY2UxLTRlOTQtODBjMi0xOGUyM2JiZDcyNGE= #ecr:eu-west-1:86c8f5ec-1ce1-4e94-80c2-18e23bbd724a
m: R0FIVDEyMzQ1Njc4 #GAHT12345678
2 changes: 1 addition & 1 deletion ...netes_credentials_provider/convertors/AWSCredentialsConvertorTest/validMissingIamMfa.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ data:
accessKey: QUJDMTIzNDU2 #ABC123456
secretKey: UGEkJHdvcmQ= #Pa$$word
iamRoleArn: ZWNyOmV1LXdlc3QtMTo4NmM4ZjVlYy0xY2UxLTRlOTQtODBjMi0xOGUyM2JiZDcyNGE= #ecr:eu-west-1:86c8f5ec-1ce1-4e94-80c2-18e23bbd724a

iamExternalId: ZXh0LWlk #ext-id
1 change: 1 addition & 0 deletions ...etes_credentials_provider/convertors/AWSCredentialsConvertorTest/validMissingIamRole.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ data:
accessKey: QUJDMTIzNDU2 #ABC123456
secretKey: UGEkJHdvcmQ= #Pa$$word
iamMfaSerialNumber: R0FIVDEyMzQ1Njc4 #GAHT12345678
iamExternalId: ZXh0LWlk #ext-id

1 change: 1 addition & 0 deletions ...s/kubernetes_credentials_provider/convertors/AWSCredentialsConvertorTest/validScoped.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ data:
secretKey: UGEkJHdvcmQ= #Pa$$word
iamRoleArn: ZWNyOmV1LXdlc3QtMTo4NmM4ZjVlYy0xY2UxLTRlOTQtODBjMi0xOGUyM2JiZDcyNGE= #ecr:eu-west-1:86c8f5ec-1ce1-4e94-80c2-18e23bbd724a
iamMfaSerialNumber: R0FIVDEyMzQ1Njc4 #GAHT12345678
iamExternalId: ZXh0LWlk #ext-id