Skip to content
This repository has been archived by the owner on Feb 5, 2021. It is now read-only.

Add STS namespace for issuer #99

Merged
merged 2 commits into from
Oct 28, 2019
Merged

Add STS namespace for issuer #99

merged 2 commits into from
Oct 28, 2019

Conversation

tharindulak
Copy link
Member

Purpose

Add STS namespace for Issuer's name.

@@ -101,7 +101,11 @@ public static String getPrettyPrintJson(Map<String, String> attributes) {
*/
public static String getIssuerName(String cellName) {

return cellName + "--sts-service";
if (cellName.equals("composite")) {
Copy link
Member

Choose a reason for hiding this comment

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

There is an already existing constant for this. Can we reuse this ?

@@ -156,7 +156,7 @@ public void testIsWorkloadExternalToCellery(String workload) throws Exception {
@Test
public void testGetIssuerName() throws Exception {

Assert.assertEquals(CellStsUtils.getIssuerName("hr"), "hr--sts-service");
Assert.assertEquals(CellStsUtils.getIssuerName("hr"), "hr--sts-service.default");
Copy link
Member

Choose a reason for hiding this comment

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

Can we use the constants we defined ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@@ -101,7 +101,11 @@ public static String getPrettyPrintJson(Map<String, String> attributes) {
*/
public static String getIssuerName(String cellName) {

return cellName + "--sts-service";
if (cellName.equals(Constants.COMPOSITE_CELL_NAME)) {
return cellName + Constants.STS_SERVICE + "." + Constants.SYSTEM_NAMESPACE;
Copy link
Member

Choose a reason for hiding this comment

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

Can we use StringBuilder for these concatenation ? It will be better.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants