change/opensearch_config_cleanup (#3701)

* Common metadata for AZURE_APP_CLIENT_[ID|SECRET], mostly to avoid warnings in tooling (read: IntelliJ). * Changed to PATTERN_INDICATING_VALUE_FROM_ENVIRONMENT for config set by NAIS platform. * Removed reference to unused secret teamdolly-opensearch. * - Improved error message on missing config.
navikt · Dec 17, 2024 · d7c6b12 · d7c6b12
1 parent 891f4c8
commit d7c6b12
Show file tree

Hide file tree

Showing 14 changed files with 76 additions and 44 deletions.
diff --git a/apps/dolly-backend/src/main/java/no/nav/dolly/config/OpenSearchConfig.java b/apps/dolly-backend/src/main/java/no/nav/dolly/config/OpenSearchConfig.java
@@ -20,13 +20,13 @@
 @EnableElasticsearchRepositories("no.nav.dolly.elastic")
 public class OpenSearchConfig extends AbstractOpenSearchConfiguration {
 
-    @Value("${open.search.username}")
+    @Value("${OPEN_SEARCH_USERNAME}")
     private String username;
 
-    @Value("${open.search.password}")
+    @Value("${OPEN_SEARCH_PASSWORD}")
     private String password;
 
-    @Value("${open.search.uri}")
+    @Value("${OPEN_SEARCH_URI}")
     private String uri;
 
     @Override

diff --git a/apps/dolly-backend/src/main/java/no/nav/dolly/config/OpenSearchLocalConfig.java b/apps/dolly-backend/src/main/java/no/nav/dolly/config/OpenSearchLocalConfig.java
@@ -20,7 +20,7 @@
 @EnableElasticsearchRepositories("no.nav.dolly.elastic")
 public class OpenSearchLocalConfig extends AbstractOpenSearchConfiguration {
 
-    @Value("${open.search.uri}")
+    @Value("${OPEN_SEARCH_URI}")
     private String uri;
 
     @Override

diff --git a/apps/dolly-backend/src/main/java/no/nav/dolly/elastic/consumer/ElasticParamsConsumer.java b/apps/dolly-backend/src/main/java/no/nav/dolly/elastic/consumer/ElasticParamsConsumer.java
@@ -13,18 +13,17 @@
 @Service
 public class ElasticParamsConsumer {
 
-    private WebClient webClient;
-    private String username;
-    private String password;
-    private String index;
+    private final WebClient webClient;
+    private final String username;
+    private final String password;
+    private final String index;
 
     public ElasticParamsConsumer(
             WebClient.Builder webClientBuilder,
-            @Value("${open.search.username}") String username,
-            @Value("${open.search.password}") String password,
-            @Value("${open.search.uri}") String uri,
+            @Value("${OPEN_SEARCH_USERNAME:''}") String username,
+            @Value("${OPEN_SEARCH_PASSWORD:''}") String password,
+            @Value("${OPEN_SEARCH_URI}") String uri,
             @Value("${open.search.index}") String index) {
-
         webClient = webClientBuilder
                 .baseUrl(uri)
                 .build();

diff --git a/apps/dolly-backend/src/main/resources/application-local.yml b/apps/dolly-backend/src/main/resources/application-local.yml
@@ -1,5 +1,6 @@
 AZURE_APP_CLIENT_ID: ${sm://azure-app-client-id}
 AZURE_APP_CLIENT_SECRET: ${sm://azure-app-client-secret}
+OPEN_SEARCH_URI: http://localhost:9200
 
 dolly:
   client:
@@ -29,9 +30,6 @@ management:
 
 open:
   search:
-    uri: http://localhost:9200
-    username: dummy
-    password: dummy
     index: local
 
 consumers:

diff --git a/apps/dolly-backend/src/test/resources/application-test.yml b/apps/dolly-backend/src/test/resources/application-test.yml
@@ -1,7 +1,8 @@
+OPEN_SEARCH_URI: dummy
+TOKEN_X_ISSUER: dummy
+
 APP_NAME: dolly
 APP_VERSION: ${application.version}
-environment: localhost
-TOKEN_X_ISSUER: dummy
 
 jira:
   host: http://localhost:${wiremock.server.port:0}/jira
@@ -10,11 +11,8 @@ jira:
 
 open:
   search:
-    uri: dummy
-    username: dummy
-    password: dummy
     total-fields: 150
-    index: dummy
+    index: test
 
 consumers:
   kodeverk-api:

diff --git a/apps/oppsummeringsdokument-service/config.yml b/apps/oppsummeringsdokument-service/config.yml
@@ -64,7 +64,5 @@ spec:
       memory: 1024Mi
     limits:
       memory: 2048Mi
-  envFrom:
-    - secret: teamdolly-opensearch
   ingresses:
     - "https://testnav-oppsummeringsdokument-service.intern.dev.nav.no"
diff --git a/...n/java/no/nav/testnav/apps/oppsummeringsdokumentservice/config/OpenSearchLocalConfig.java b/...n/java/no/nav/testnav/apps/oppsummeringsdokumentservice/config/OpenSearchLocalConfig.java
@@ -1,6 +1,5 @@
 package no.nav.testnav.apps.oppsummeringsdokumentservice.config;
 
-import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.opensearch.client.RestHighLevelClient;
 import org.opensearch.data.client.orhlc.AbstractOpenSearchConfiguration;
@@ -9,7 +8,6 @@
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Profile;
-import org.springframework.data.elasticsearch.repository.config.EnableElasticsearchRepositories;
 
 import java.time.Duration;
 
@@ -18,7 +16,7 @@
 @Profile("dev")
 public class OpenSearchLocalConfig extends AbstractOpenSearchConfiguration {
 
-    @Value("${open.search.uri}")
+    @Value("${OPEN_SEARCH_URI}")
     private String uri;
 
     @Override

diff --git a/...c/main/java/no/nav/testnav/apps/oppsummeringsdokumentservice/config/OpensearchConfig.java b/...c/main/java/no/nav/testnav/apps/oppsummeringsdokumentservice/config/OpensearchConfig.java
@@ -14,13 +14,13 @@
 @Configuration
 public class OpensearchConfig extends AbstractOpenSearchConfiguration {
 
-    @Value("${open.search.username}")
+    @Value("${OPEN_SEARCH_USERNAME}")
     private String username;
 
-    @Value("${open.search.password}")
+    @Value("${OPEN_SEARCH_PASSWORD}")
     private String password;
 
-    @Value("${open.search.uri}")
+    @Value("${OPEN_SEARCH_URI}")
     private String uri;
 
     @Override

diff --git a/apps/oppsummeringsdokument-service/src/main/resources/application-local.yml b/apps/oppsummeringsdokument-service/src/main/resources/application-local.yml
@@ -1,4 +1,11 @@
+AZURE_APP_CLIENT_ID: ${sm://azure-app-client-id}
+AZURE_APP_CLIENT_SECRET: ${sm://azure-app-client-secret}
+OPEN_SEARCH_URI: http://localhost:9200
+
+spring:
+  config:
+    import: "sm://"
+
 open:
   search:
-    index: local
-    uri: http://localhost:9200
+    index: local
diff --git a/.../main/java/no/nav/testnav/libs/reactivesecurity/domain/AzureNavProxyClientCredential.java b/.../main/java/no/nav/testnav/libs/reactivesecurity/domain/AzureNavProxyClientCredential.java
@@ -11,14 +11,16 @@
 @EqualsAndHashCode(callSuper = false)
 @Configuration
 public class AzureNavProxyClientCredential extends ClientCredential {
+
     private final String tokenEndpoint;
 
     public AzureNavProxyClientCredential(
             @Value("${AZURE_NAV_OPENID_CONFIG_TOKEN_ENDPOINT:#{null}}") String tokenEndpoint,
-            @Value("${AZURE_NAV_APP_CLIENT_ID:#{null}}") String clientId,
-            @Value("${AZURE_NAV_APP_CLIENT_SECRET:#{null}}") String clientSecret
+            @Value("#{systemProperties['spring.profiles.active'] == 'test' ? 'test-client-id' : '${AZURE_NAV_APP_CLIENT_ID:#{null}}'}") String clientId,
+            @Value("#{systemProperties['spring.profiles.active'] == 'test' ? 'test-client-secret' : '${AZURE_NAV_APP_CLIENT_SECRET:#{null}}'}") String clientSecret
     ) {
         super(clientId, clientSecret);
         this.tokenEndpoint = tokenEndpoint;
     }
+
 }
diff --git a/...n/java/no/nav/testnav/libs/reactivesecurity/domain/AzureTrygdeetatenClientCredential.java b/...n/java/no/nav/testnav/libs/reactivesecurity/domain/AzureTrygdeetatenClientCredential.java
@@ -1,5 +1,6 @@
 package no.nav.testnav.libs.reactivesecurity.domain;
 
+import lombok.Getter;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.annotation.Configuration;
 
@@ -8,23 +9,20 @@
 import no.nav.testnav.libs.securitycore.domain.azuread.ClientCredential;
 
 @Configuration
+@Getter
 public class AzureTrygdeetatenClientCredential extends ClientCredential {
+
     private final String tokenEndpoint;
 
     public AzureTrygdeetatenClientCredential(
             @Value("${AZURE_TRYGDEETATEN_OPENID_CONFIG_TOKEN_ENDPOINT:#{null}}") String tokenEndpoint,
-            @Value("${AZURE_TRYGDEETATEN_APP_CLIENT_ID:#{null}}") String clientId,
-            @Value("${AZURE_TRYGDEETATEN_APP_CLIENT_SECRET:#{null}}") String clientSecret
+            @Value("#{systemProperties['spring.profiles.active'] == 'test' ? 'test-client-id' : '${AZURE_TRYGDEETATEN_APP_CLIENT_ID:#{null}}'}") String clientId,
+            @Value("#{systemProperties['spring.profiles.active'] == 'test' ? 'test-client-secret' : '${AZURE_TRYGDEETATEN_CLIENT_SECRET:#{null}}'}") String clientSecret
     ) {
         super(clientId, clientSecret);
         this.tokenEndpoint = tokenEndpoint;
     }
 
-    public String getTokenEndpoint() {
-        return tokenEndpoint;
-    }
-
-
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
@@ -38,4 +36,5 @@ public boolean equals(Object o) {
     public int hashCode() {
         return Objects.hash(super.hashCode(), tokenEndpoint);
     }
+
 }
diff --git a/...c/main/java/no/nav/testnav/libs/securitycore/domain/azuread/AzureNavClientCredential.java b/...c/main/java/no/nav/testnav/libs/securitycore/domain/azuread/AzureNavClientCredential.java
@@ -6,9 +6,16 @@
 @Configuration
 public class AzureNavClientCredential extends ClientCredential {
 
+    /*
+    TODO: A better solution, for another day:
+        1. No longer import AzureNavClientCredential - generify this, or use factories. It cannot be a @Configuration for its own @Bean.
+        2. Create two beans, one for @Profile("test"), one on @ConditionalOnMissingBean.
+        3. Check if we really need subclasses for this, or if ClientCredential will suffice. Check all other extends ClientCredential.
+        This class is effectively both a @Configuration and a @Bean with immutable config, which doesn't play nice.
+     */
     public AzureNavClientCredential(
-            @Value("${AZURE_APP_CLIENT_ID:#{null}}") String clientId,
-            @Value("${AZURE_APP_CLIENT_SECRET:#{null}}") String clientSecret
+            @Value("#{systemProperties['spring.profiles.active'] == 'test' ? 'test-client-id' : '${AZURE_APP_CLIENT_ID:#{null}}'}") String clientId,
+            @Value("#{systemProperties['spring.profiles.active'] == 'test' ? 'test-client-secret' : '${AZURE_APP_CLIENT_SECRET:#{null}}'}") String clientSecret
     ) {
         super(clientId, clientSecret);
     }

diff --git a/...-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/ClientCredential.java b/...-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/ClientCredential.java
@@ -1,12 +1,25 @@
 package no.nav.testnav.libs.securitycore.domain.azuread;
 
-import lombok.Data;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import org.springframework.util.Assert;
 
-@Data
+@EqualsAndHashCode
+@Getter
 public class ClientCredential {
+
+    private static final String MISSING_CONFIG_MESSAGE = "AZURE_NAV_APP_CLIENT_ID and AZURE_NAV_APP_CLIENT_SECRET must be set";
+
     private final String clientId;
     private final String clientSecret;
 
+    public ClientCredential(String clientId, String clientSecret) {
+        Assert.noNullElements(new String[]{clientId, clientSecret}, MISSING_CONFIG_MESSAGE);
+
+        this.clientId = clientId;
+        this.clientSecret = clientSecret;
+    }
+
     @Override
     public final String toString() {
         return "ClientCredential{" +

diff --git a/libs/security-core/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/libs/security-core/src/main/resources/META-INF/additional-spring-configuration-metadata.json
@@ -0,0 +1,13 @@
+{
+  "properties": [
+    {
+      "name": "AZURE_APP_CLIENT_ID",
+      "type": "java.lang.String",
+      "description": "Settes av NAIS i pod, må overstyres med secret ved kjøring lokalt."
+    },
+    {
+      "name": "AZURE_APP_CLIENT_SECRET",
+      "type": "java.lang.String",
+      "description": "Settes av NAIS i pod, må overstyres med secret ved kjøring lokalt."
+    }
+] }