Merge pull request #4025 from vespa-engine/bjorncs/athenz-identity-provider-cleanup

Bjorncs/athenz identity provider cleanup

8 files changed, 32 insertions, 32 deletions

diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/AthenzCredentialsService.java b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/AthenzCredentialsService.java
index ea9e50cbb95..5786eb9e398 100644
--- a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/AthenzCredentialsService.java
+++ b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/AthenzCredentialsService.java
@@ -19,23 +19,23 @@ class AthenzCredentialsService {
     private static final ObjectMapper mapper = new ObjectMapper();
 
     private final IdentityConfig identityConfig;
-    private final ServiceProviderApi serviceProviderApi;
+    private final IdentityDocumentService identityDocumentService;
     private final AthenzService athenzService;
     private final Clock clock;
 
     AthenzCredentialsService(IdentityConfig identityConfig,
-                             ServiceProviderApi serviceProviderApi,
+                             IdentityDocumentService identityDocumentService,
                              AthenzService athenzService,
                              Clock clock) {
         this.identityConfig = identityConfig;
-        this.serviceProviderApi = serviceProviderApi;
+        this.identityDocumentService = identityDocumentService;
         this.athenzService = athenzService;
         this.clock = clock;
     }
 
     AthenzCredentials registerInstance() {
         KeyPair keyPair = CryptoUtils.createKeyPair();
-        String rawDocument = serviceProviderApi.getSignedIdentityDocument();
+        String rawDocument = identityDocumentService.getSignedIdentityDocument();
         SignedIdentityDocument document = parseSignedIdentityDocument(rawDocument);
         PKCS10CertificationRequest csr = CryptoUtils.createCSR(identityConfig.domain(),
                                                                identityConfig.service(),
@@ -47,8 +47,7 @@ class AthenzCredentialsService {
                                                 identityConfig.domain(),
                                                 identityConfig.service(),
                                                 rawDocument,
-                                                CryptoUtils.toPem(csr),
-                                                true);
+                                                CryptoUtils.toPem(csr));
         InstanceIdentity instanceIdentity = athenzService.sendInstanceRegisterRequest(instanceRegisterInformation,
                                                                                       document.ztsEndpoint);
         return toAthenzCredentials(instanceIdentity, keyPair, document);
@@ -62,8 +61,7 @@ class AthenzCredentialsService {
                                                                document.dnsSuffix,
                                                                document.providerUniqueId,
                                                                newKeyPair);
-        InstanceRefreshInformation refreshInfo = new InstanceRefreshInformation(CryptoUtils.toPem(csr),
-                                                                                /*requestServiceToken*/true);
+        InstanceRefreshInformation refreshInfo = new InstanceRefreshInformation(CryptoUtils.toPem(csr));
         InstanceIdentity instanceIdentity =
                 athenzService.sendInstanceRefreshRequest(document.providerService,
                                                          identityConfig.domain(),
diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/AthenzIdentityProviderImpl.java b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/AthenzIdentityProviderImpl.java
index 2f98d852a95..356780a0900 100644
--- a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/AthenzIdentityProviderImpl.java
+++ b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/AthenzIdentityProviderImpl.java
@@ -57,7 +57,7 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen
     public AthenzIdentityProviderImpl(IdentityConfig config) {
         this(config,
              new AthenzCredentialsService(config,
-                                          new ServiceProviderApi(config.loadBalancerAddress()),
+                                          new IdentityDocumentService(config.loadBalancerAddress()),
                                           new AthenzService(),
                                           Clock.systemUTC()),
              new ThreadPoolScheduler(),
@@ -164,7 +164,7 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen
                 credentials.set(newCredentials);
                 scheduler.schedule(new UpdateCredentialsTask(), UPDATE_PERIOD);
             } catch (Throwable t) {
-                log.log(LogLevel.ERROR, "Failed to update credentials: " + t.getMessage(), t);
+                log.log(LogLevel.WARNING, "Failed to update credentials: " + t.getMessage(), t);
                 lastThrowable.set(t);
                 Duration timeToExpiration = Duration.between(clock.instant(), getExpirationTime(currentCredentials));
                 // NOTE: Update period might be after timeToExpiration, still we do not want to DDoS Athenz.
diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/AthenzService.java b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/AthenzService.java
index 4c1b603e859..898f90e3438 100644
--- a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/AthenzService.java
+++ b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/AthenzService.java
@@ -36,7 +36,7 @@ import java.security.cert.X509Certificate;
  */
 public class AthenzService {
 
-    private static final String INSTANCE_API_PATH = "zts/v1/instance";
+    private static final String INSTANCE_API_PATH = "/zts/v1/instance";
 
     private final ObjectMapper objectMapper = new ObjectMapper();
     private final HttpRequestRetryHandler retryHandler = new DefaultHttpRequestRetryHandler(3, /*requestSentRetryEnabled*/true);
@@ -66,11 +66,14 @@ public class AthenzService {
                                                        X509Certificate certicate,
                                                        PrivateKey privateKey) {
         try (CloseableHttpClient client = createHttpClientWithTlsAuth(certicate, privateKey, retryHandler)) {
-            String uriPath = String.format(
-                    "%s/%s/%s/%s/%s",
-                    INSTANCE_API_PATH, providerService, instanceDomain, instanceServiceName, instanceId);
+            URI uri = ztsEndpoint
+                    .resolve(INSTANCE_API_PATH + '/')
+                    .resolve(providerService + '/')
+                    .resolve(instanceDomain + '/')
+                    .resolve(instanceServiceName + '/')
+                    .resolve(instanceId);
             HttpUriRequest postRequest = RequestBuilder.post()
-                    .setUri(ztsEndpoint.resolve(uriPath))
+                    .setUri(uri)
                     .setEntity(toJsonStringEntity(instanceRefreshInformation))
                     .build();
             return getInstanceIdentity(client, postRequest);
diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/ServiceProviderApi.java b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/IdentityDocumentService.java
index 3bd0cae71eb..c524ef790d4 100644
--- a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/ServiceProviderApi.java
+++ b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/IdentityDocumentService.java
@@ -24,12 +24,12 @@ import java.security.NoSuchAlgorithmException;
  * @author mortent
  * @author bjorncs
  */
-public class ServiceProviderApi {
+public class IdentityDocumentService {
 
     private final URI identityDocumentApiUri;
 
-    public ServiceProviderApi(String configServerHostname) {
-        this.identityDocumentApiUri = createIdentityDocumentApiUri(configServerHostname);
+    public IdentityDocumentService(String loadBalancerName) {
+        this.identityDocumentApiUri = createIdentityDocumentApiUri(loadBalancerName);
     }
 
     /**
@@ -67,12 +67,12 @@ public class ServiceProviderApi {
         }
     }
 
-    private static URI createIdentityDocumentApiUri(String providerHostname) {
+    private static URI createIdentityDocumentApiUri(String loadBalancerName) {
         try {
             // TODO Figure out a proper way of determining the hostname matching what's registred in node-repository
             return new URIBuilder()
                     .setScheme("https")
-                    .setHost(providerHostname)
+                    .setHost(loadBalancerName)
                     .setPort(8443)
                     .setPath("/athenz/v1/provider/identity-document")
                     .addParameter("hostname", Defaults.getDefaults().vespaHostname())
diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/InstanceRefreshInformation.java b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/InstanceRefreshInformation.java
index 621eafca3bb..dd893cb3143 100644
--- a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/InstanceRefreshInformation.java
+++ b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/InstanceRefreshInformation.java
@@ -15,10 +15,9 @@ public class InstanceRefreshInformation {
     @JsonProperty("csr")
     private final String csr;
     @JsonProperty("token")
-    private final boolean requestServiceToken;
+    private final boolean requestServiceToken = true;
 
-    public InstanceRefreshInformation(String csr, boolean requestServiceToken) {
+    public InstanceRefreshInformation(String csr) {
         this.csr = csr;
-        this.requestServiceToken = requestServiceToken;
     }
 }
diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/InstanceRegisterInformation.java b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/InstanceRegisterInformation.java
index 61ab810abd5..e2355cb7a2d 100644
--- a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/InstanceRegisterInformation.java
+++ b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/InstanceRegisterInformation.java
@@ -26,14 +26,13 @@ public class InstanceRegisterInformation {
     @JsonProperty("csr")
     private final String csr;
     @JsonProperty("token")
-    private final boolean token;
+    private final boolean token = true;
 
-    public InstanceRegisterInformation(String provider, String domain, String service, String attestationData, String csr, boolean token) {
+    public InstanceRegisterInformation(String provider, String domain, String service, String attestationData, String csr) {
         this.provider = provider;
         this.domain = domain;
         this.service = service;
         this.attestationData = attestationData;
         this.csr = csr;
-        this.token = token;
     }
 }
diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/SignedIdentityDocument.java b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/SignedIdentityDocument.java
index d9b9bdd5c0d..5d5b5430859 100644
--- a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/SignedIdentityDocument.java
+++ b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/impl/SignedIdentityDocument.java
@@ -10,6 +10,7 @@ import java.net.URI;
 /**
  * @author bjorncs
  */
+// TODO Most of these value should ideally be config provided by config-model
 @JsonIgnoreProperties(ignoreUnknown = true)
 @JsonInclude(JsonInclude.Include.NON_NULL)
 class SignedIdentityDocument {
diff --git a/container-disc/src/test/java/com/yahoo/container/jdisc/athenz/impl/AthenzIdentityProviderImplTest.java b/container-disc/src/test/java/com/yahoo/container/jdisc/athenz/impl/AthenzIdentityProviderImplTest.java
index fd38e589d10..1c0efef2089 100644
--- a/container-disc/src/test/java/com/yahoo/container/jdisc/athenz/impl/AthenzIdentityProviderImplTest.java
+++ b/container-disc/src/test/java/com/yahoo/container/jdisc/athenz/impl/AthenzIdentityProviderImplTest.java
@@ -44,16 +44,16 @@ public class AthenzIdentityProviderImplTest {
 
     @Test
     public void athenz_credentials_are_retrieved_after_component_contruction_completed() {
-        ServiceProviderApi serviceProviderApi = mock(ServiceProviderApi.class);
+        IdentityDocumentService identityDocumentService = mock(IdentityDocumentService.class);
         AthenzService athenzService = mock(AthenzService.class);
         ManualClock clock = new ManualClock(Instant.EPOCH);
         MockScheduler scheduler = new MockScheduler(clock);
 
-        when(serviceProviderApi.getSignedIdentityDocument()).thenReturn(getIdentityDocument());
+        when(identityDocumentService.getSignedIdentityDocument()).thenReturn(getIdentityDocument());
         when(athenzService.sendInstanceRegisterRequest(any(), any())).thenReturn(
                 new InstanceIdentity(null, "TOKEN"));
         AthenzCredentialsService credentialService =
-                new AthenzCredentialsService(IDENTITY_CONFIG, serviceProviderApi, athenzService, clock);
+                new AthenzCredentialsService(IDENTITY_CONFIG, identityDocumentService, athenzService, clock);
 
         AthenzIdentityProvider identityProvider =
                 new AthenzIdentityProviderImpl(IDENTITY_CONFIG, credentialService, scheduler, clock);
@@ -103,12 +103,12 @@ public class AthenzIdentityProviderImplTest {
 
     @Test
     public void failed_credentials_updates_will_schedule_retries() {
-        ServiceProviderApi serviceProviderApi = mock(ServiceProviderApi.class);
+        IdentityDocumentService identityDocumentService = mock(IdentityDocumentService.class);
         AthenzService athenzService = mock(AthenzService.class);
         ManualClock clock = new ManualClock(Instant.EPOCH);
         MockScheduler scheduler = new MockScheduler(clock);
 
-        when(serviceProviderApi.getSignedIdentityDocument()).thenReturn(getIdentityDocument());
+        when(identityDocumentService.getSignedIdentityDocument()).thenReturn(getIdentityDocument());
         when(athenzService.sendInstanceRegisterRequest(any(), any())).thenReturn(
                 new InstanceIdentity(null, "TOKEN"));
         when(athenzService.sendInstanceRefreshRequest(anyString(), anyString(), anyString(),
@@ -118,7 +118,7 @@ public class AthenzIdentityProviderImplTest {
                 .thenThrow(new RuntimeException("#3"))
                 .thenReturn(new InstanceIdentity(null, "TOKEN"));
         AthenzCredentialsService credentialService =
-                new AthenzCredentialsService(IDENTITY_CONFIG, serviceProviderApi, athenzService, clock);
+                new AthenzCredentialsService(IDENTITY_CONFIG, identityDocumentService, athenzService, clock);
 
         AthenzIdentityProvider identityProvider =
                 new AthenzIdentityProviderImpl(IDENTITY_CONFIG, credentialService, scheduler, clock);