Skip to content

Commit 7476158

Browse files
rjeberhardrjeberhard
authored
Merge pull request #1034 from oracle/owls-72193
OWLS-72193 - Operator logs repeated NPE when a domain resource credential secret is missing
2 parents d461d8e + 99e9c70 commit 7476158

17 files changed

+685
-83
lines changed

‎operator/src/main/java/oracle/kubernetes/operator/DomainProcessorImpl.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@
3535
import oracle.kubernetes.operator.helpers.ServiceHelper;
3636
import oracle.kubernetes.operator.logging.LoggingFacade;
3737
import oracle.kubernetes.operator.logging.LoggingFactory;
38+
import oracle.kubernetes.operator.logging.LoggingFilter;
3839
import oracle.kubernetes.operator.logging.MessageKeys;
40+
import oracle.kubernetes.operator.logging.OncePerMessageLoggingFilter;
3941
import oracle.kubernetes.operator.steps.BeforeAdminServiceStep;
4042
import oracle.kubernetes.operator.steps.DeleteDomainStep;
4143
import oracle.kubernetes.operator.steps.DomainPresenceStep;
@@ -290,6 +292,7 @@ public void dispatchDomainWatch(Watch.Response<Domain> item) {
290292

291293
private void scheduleDomainStatusUpdating(DomainPresenceInfo info) {
292294
AtomicInteger unchangedCount = new AtomicInteger(0);
295+
final OncePerMessageLoggingFilter loggingFilter = new OncePerMessageLoggingFilter();
293296
Runnable command =
294297
new Runnable() {
295298
public void run() {
@@ -301,6 +304,7 @@ public void run() {
301304
.put(
302305
ProcessingConstants.DOMAIN_COMPONENT_NAME,
303306
Component.createFor(info, delegate.getVersion()));
307+
packet.put(LoggingFilter.LOGGING_FILTER_PACKET_KEY, loggingFilter);
304308
MainTuning main = TuningParameters.getInstance().getMainTuning();
305309
Step strategy =
306310
DomainStatusUpdater.createStatusStep(main.statusUpdateTimeoutSeconds, null);
@@ -312,6 +316,11 @@ public void run() {
312316
new CompletionCallback() {
313317
@Override
314318
public void onCompletion(Packet packet) {
319+
if (Boolean.TRUE.equals(packet.get(ProcessingConstants.SERVER_HEALTH_READ))) {
320+
loggingFilter.setFiltering(false).resetLogHistory();
321+
} else {
322+
loggingFilter.setFiltering(true);
323+
}
315324
Boolean isStatusUnchanged =
316325
(Boolean) packet.get(ProcessingConstants.STATUS_UNCHANGED);
317326
if (Boolean.TRUE.equals(isStatusUnchanged)) {
@@ -344,6 +353,7 @@ public void onCompletion(Packet packet) {
344353
@Override
345354
public void onThrowable(Packet packet, Throwable throwable) {
346355
LOGGER.severe(MessageKeys.EXCEPTION, throwable);
356+
loggingFilter.setFiltering(true);
347357
// retry to trying after shorter delay because of exception
348358
unchangedCount.set(0);
349359
registerStatusUpdater(

‎operator/src/main/java/oracle/kubernetes/operator/ProcessingConstants.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,6 @@ public interface ProcessingConstants {
3333
public static final String DOMAIN_INTROSPECTOR_JOB = "domainIntrospectorJob";
3434
public static final String DOMAIN_INTROSPECTOR_LOG_RESULT = "domainIntrospectorLogResult";
3535
public static final String SIT_CONFIG_MAP = "sitConfigMap";
36+
37+
public static final String SERVER_HEALTH_READ = "serverHealthRead";
3638
}

‎operator/src/main/java/oracle/kubernetes/operator/helpers/SecretHelper.java

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import java.util.Map;
1212
import oracle.kubernetes.operator.logging.LoggingFacade;
1313
import oracle.kubernetes.operator.logging.LoggingFactory;
14+
import oracle.kubernetes.operator.logging.LoggingFilter;
1415
import oracle.kubernetes.operator.logging.MessageKeys;
1516
import oracle.kubernetes.operator.work.ContainerResolver;
1617
import oracle.kubernetes.operator.work.NextAction;
@@ -72,7 +73,7 @@ public Map<String, byte[]> getSecretData(SecretType secretType, String secretNam
7273
return null;
7374
}
7475

75-
return harvestAdminSecretData(secret);
76+
return harvestAdminSecretData(secret, null);
7677
} catch (Throwable e) {
7778
LOGGER.severe(MessageKeys.EXCEPTION, e);
7879
return null;
@@ -116,6 +117,7 @@ public NextAction apply(Packet packet) {
116117
}
117118

118119
LOGGER.fine(MessageKeys.RETRIEVING_SECRET, secretName);
120+
final LoggingFilter loggingFilter = packet.getValue(LoggingFilter.LOGGING_FILTER_PACKET_KEY);
119121
CallBuilderFactory factory =
120122
ContainerResolver.getInstance().getContainer().getSPI(CallBuilderFactory.class);
121123
Step read =
@@ -132,7 +134,7 @@ public NextAction onFailure(
132134
int statusCode,
133135
Map<String, List<String>> responseHeaders) {
134136
if (statusCode == CallBuilder.NOT_FOUND) {
135-
LOGGER.warning(MessageKeys.SECRET_NOT_FOUND, secretName);
137+
LOGGER.warning(loggingFilter, MessageKeys.SECRET_NOT_FOUND, secretName);
136138
return doNext(packet);
137139
}
138140
return super.onFailure(packet, e, statusCode, responseHeaders);
@@ -144,7 +146,7 @@ public NextAction onSuccess(
144146
V1Secret result,
145147
int statusCode,
146148
Map<String, List<String>> responseHeaders) {
147-
packet.put(SECRET_DATA_KEY, harvestAdminSecretData(result));
149+
packet.put(SECRET_DATA_KEY, harvestAdminSecretData(result, loggingFilter));
148150
return doNext(packet);
149151
}
150152
});
@@ -153,21 +155,24 @@ public NextAction onSuccess(
153155
}
154156
}
155157

156-
private static Map<String, byte[]> harvestAdminSecretData(V1Secret secret) {
158+
private static Map<String, byte[]> harvestAdminSecretData(
159+
V1Secret secret, LoggingFilter loggingFilter) {
157160
Map<String, byte[]> secretData = new HashMap<>();
158161
byte[] usernameBytes = secret.getData().get(ADMIN_SERVER_CREDENTIALS_USERNAME);
159162
byte[] passwordBytes = secret.getData().get(ADMIN_SERVER_CREDENTIALS_PASSWORD);
160163

161164
if (usernameBytes != null) {
162165
secretData.put(ADMIN_SERVER_CREDENTIALS_USERNAME, usernameBytes);
163166
} else {
164-
LOGGER.warning(MessageKeys.SECRET_DATA_NOT_FOUND, ADMIN_SERVER_CREDENTIALS_USERNAME);
167+
LOGGER.warning(
168+
loggingFilter, MessageKeys.SECRET_DATA_NOT_FOUND, ADMIN_SERVER_CREDENTIALS_USERNAME);
165169
}
166170

167171
if (passwordBytes != null) {
168172
secretData.put(ADMIN_SERVER_CREDENTIALS_PASSWORD, passwordBytes);
169173
} else {
170-
LOGGER.warning(MessageKeys.SECRET_DATA_NOT_FOUND, ADMIN_SERVER_CREDENTIALS_PASSWORD);
174+
LOGGER.warning(
175+
loggingFilter, MessageKeys.SECRET_DATA_NOT_FOUND, ADMIN_SERVER_CREDENTIALS_PASSWORD);
171176
}
172177
return secretData;
173178
}

‎operator/src/main/java/oracle/kubernetes/operator/http/HttpClient.java

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,15 +196,23 @@ public NextAction apply(Packet packet) {
196196
if (secretData != null) {
197197
username = secretData.get(SecretHelper.ADMIN_SERVER_CREDENTIALS_USERNAME);
198198
password = secretData.get(SecretHelper.ADMIN_SERVER_CREDENTIALS_PASSWORD);
199-
}
200-
packet.put(KEY, createAuthenticatedClient(username, password));
199+
packet.put(KEY, createAuthenticatedClient(username, password));
201200

202-
Arrays.fill(username, (byte) 0);
203-
Arrays.fill(password, (byte) 0);
201+
clearCredential(username);
202+
clearCredential(password);
203+
}
204204
return doNext(packet);
205205
}
206206
}
207207

208+
/**
209+
* Erase authentication credential so that it is not sitting in memory where a rogue program can
210+
* find it.
211+
*/
212+
private static void clearCredential(byte[] credential) {
213+
if (credential != null) Arrays.fill(credential, (byte) 0);
214+
}
215+
208216
/**
209217
* Create authenticated client specifically targeted at an admin server.
210218
*

‎operator/src/main/java/oracle/kubernetes/operator/logging/LoggingFacade.java

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,20 @@ public void info(String msg, Object... params) {
292292
}
293293
}
294294

295+
/**
296+
* Logs a message which requires parameters at the INFO level with a logging filter applied
297+
*
298+
* @param loggingFilter LoggingFilter to be applied, can be null
299+
* @param msg the message to log
300+
* @param params varargs list of objects to include in the log message
301+
*/
302+
public void info(LoggingFilter loggingFilter, String msg, Object... params) {
303+
if (isInfoEnabled() && LoggingFilter.canLog(loggingFilter, msg)) {
304+
CallerDetails details = inferCaller();
305+
logger.logp(Level.INFO, details.clazz, details.method, msg, params);
306+
}
307+
}
308+
295309
/**
296310
* Logs a message which accompanies a Throwable at the INFO level.
297311
*
@@ -449,6 +463,20 @@ public void severe(String msg, Object... params) {
449463
}
450464
}
451465

466+
/**
467+
* Logs a message which requires parameters at the SEVERE level with a logging filter applied
468+
*
469+
* @param loggingFilter LoggingFilter to be applied, can be null
470+
* @param msg the message to log
471+
* @param params varargs list of objects to include in the log message
472+
*/
473+
public void severe(LoggingFilter loggingFilter, String msg, Object... params) {
474+
if (isSevereEnabled() && LoggingFilter.canLog(loggingFilter, msg)) {
475+
CallerDetails details = inferCaller();
476+
logger.logp(Level.SEVERE, details.clazz, details.method, msg, params);
477+
}
478+
}
479+
452480
/**
453481
* Logs a message which accompanies a Throwable at the SEVERE level.
454482
*
@@ -462,6 +490,20 @@ public void severe(String msg, Throwable thrown) {
462490
}
463491
}
464492

493+
/**
494+
* Logs a message which accompanies a Throwable at the SEVERE level with a logging filter applied
495+
*
496+
* @param loggingFilter LoggingFilter to be applied, can be null
497+
* @param msg the message to log
498+
* @param thrown an Exception to include in the logged message
499+
*/
500+
public void severe(LoggingFilter loggingFilter, String msg, Throwable thrown) {
501+
if (isSevereEnabled() && LoggingFilter.canLog(loggingFilter, msg)) {
502+
CallerDetails details = inferCaller();
503+
logger.logp(Level.SEVERE, details.clazz, details.method, msg, thrown);
504+
}
505+
}
506+
465507
/**
466508
* Logs that an exception will be thrown. The calling class and method names will be inferred.
467509
*
@@ -499,6 +541,20 @@ public void warning(String msg, Object... params) {
499541
}
500542
}
501543

544+
/**
545+
* Logs a message which requires parameters at the WARNING level with a logging filter applied
546+
*
547+
* @param loggingFilter LoggingFilter to be applied, can be null
548+
* @param msg the message to log
549+
* @param params varargs list of objects to include in the log message
550+
*/
551+
public void warning(LoggingFilter loggingFilter, String msg, Object... params) {
552+
if (isWarningEnabled() && LoggingFilter.canLog(loggingFilter, msg)) {
553+
CallerDetails details = inferCaller();
554+
logger.logp(Level.WARNING, details.clazz, details.method, msg, params);
555+
}
556+
}
557+
502558
/**
503559
* Logs a message which accompanies a Throwable at the WARNING level.
504560
*

‎operator/src/main/java/oracle/kubernetes/operator/logging/LoggingFilter.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright 2019, Oracle Corporation and/or its affiliates. All rights reserved.
2+
// Licensed under the Universal Permissive License v 1.0 as shown at
3+
// http://oss.oracle.com/licenses/upl.
4+
5+
package oracle.kubernetes.operator.logging;
6+
7+
/** A filter to control whether a log message should be logged */
8+
public interface LoggingFilter {
9+
10+
// Constant for key for storing LoggingFilter object in Packet map
11+
String LOGGING_FILTER_PACKET_KEY = "loggingFilter";
12+
13+
/**
14+
* Checks if the message should be logged
15+
*
16+
* @param msg the message to be loggged
17+
*/
18+
boolean canLog(String msg);
19+
20+
/**
21+
* Checks if the message should be logged according to an optional LoggingFilter
22+
*
23+
* @param loggingFilter LoggingFilter that decides whether the log message should be logged, can
24+
* be null
25+
* @param msg The log message to be loggd
26+
* @return the canLog() return value from the provided loggingFilter, message, and parameters, or
27+
* true if loggingFilter is null
28+
*/
29+
static boolean canLog(LoggingFilter loggingFilter, String msg) {
30+
return loggingFilter == null || loggingFilter.canLog(msg);
31+
}
32+
}

‎operator/src/main/java/oracle/kubernetes/operator/logging/MessageKeys.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,4 +154,5 @@ private MessageKeys() {}
154154
public static final String EXTERNAL_CHANNEL_SERVICE_CREATED = "WLSKO-0150";
155155
public static final String EXTERNAL_CHANNEL_SERVICE_REPLACED = "WLSKO-0151";
156156
public static final String EXTERNAL_CHANNEL_SERVICE_EXISTS = "WLSKO-0152";
157+
public static final String WLS_HEALTH_READ_FAILED_NO_HTTPCLIENT = "WLSKO-0153";
157158
}

‎operator/src/main/java/oracle/kubernetes/operator/logging/OncePerMessageLoggingFilter.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// Copyright 2019, Oracle Corporation and/or its affiliates. All rights reserved.
2+
// Licensed under the Universal Permissive License v 1.0 as shown at
3+
// http://oss.oracle.com/licenses/upl.
4+
5+
package oracle.kubernetes.operator.logging;
6+
7+
import java.util.HashSet;
8+
import java.util.Set;
9+
10+
/** A LoggingFilter that logs each log message, which are typically message keys, at most once */
11+
public class OncePerMessageLoggingFilter implements LoggingFilter {
12+
13+
// allow all messages to be logged when filtering is off
14+
boolean filtering = false;
15+
16+
Set messagesLogged = new HashSet();
17+
18+
/**
19+
* Turn on or off the filtering of log messages and skip logging of messages that have already
20+
* been logged
21+
*
22+
* @param value true if filtering should be on, false if filtering should be off
23+
*/
24+
public synchronized OncePerMessageLoggingFilter setFiltering(boolean value) {
25+
filtering = value;
26+
return this;
27+
}
28+
29+
/** Clears the list of history of messages logged and turn off filtering */
30+
public synchronized OncePerMessageLoggingFilter resetLogHistory() {
31+
messagesLogged.clear();
32+
return this;
33+
}
34+
35+
@Override
36+
public synchronized boolean canLog(String msg) {
37+
// Do not log if filtering is on and message has already been logged
38+
if (filtering && messagesLogged.contains(msg)) {
39+
return false;
40+
}
41+
messagesLogged.add(msg);
42+
return true;
43+
}
44+
}

0 commit comments

Comments
 (0)