Skip to content

Improve JWK messages #807

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

Open
1000TurquoisePogs wants to merge 4 commits into
base: v3.x/staging
Choose a base branch
from
Open

Improve JWK messages #807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d48d9ed
Improve JWK messages
1000TurquoisePogs Nov 19, 2025
b720d89
Update CHANGELOG.md
1000TurquoisePogs Nov 19, 2025
b977b89
TLS error is a RSN not an RC
1000TurquoisePogs Nov 19, 2025
fc0a199
Merge branch 'v3.x/staging' into bugfix/v3/better-jwk-troubleshooting
Martin-Zeithaml Jan 22, 2026
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

All notable changes to the ZSS package will be documented in this file.

## `3.5.0`
- Bugfix: Common JWK error messages contain more information about how to further diagnose their cause. [(#807)](https://github.com/zowe/zss/pull/807)

## `3.4.0`
- Bugfix: Fixed hostname to IP address lookup for "bind-test" program. [(#801)](https://github.com/zowe/zss/pull/801)

Expand Down
16 changes: 15 additions & 1 deletion c/jwk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,22 @@ static int jwkTaskMain(RLETask *task) {
if ((i+1) % warnInterval == 0) {
zowelog(NULL, LOG_COMP_ID_JWK, ZOWE_LOG_WARNING, ZSS_LOG_JWK_RETRY_MSG,
jwkGetStrStatus(rc), rc, jwkHttpClientGetStrStatus(rsn), rsn, retryIntervalSeconds);
if (rsn == HTTP_CLIENT_TLS_ERROR) {
zowelog(NULL, LOG_COMP_ID_JWK, ZOWE_LOG_WARNING, "If TLS error persists, trace GSK for more detail using YAML property 'components.zss.agent.https.trace: true'\n");
}
}
sleep(retryIntervalSeconds);
}
}
if (success) {
zowelog(NULL, LOG_COMP_ID_JWK, ZOWE_LOG_INFO, ZSS_LOG_JWK_READY_MSG, settings->fallback ? "with" : "without");
} else {
Copy link
Copy Markdown
Contributor

@ifakhrutdinov ifakhrutdinov Nov 20, 2025

Choose a reason for hiding this comment

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

I'd add a helper function and simplify this code:

static char *printJWKURL(const JwkSettings *settings, char *buffer, size_t bufferLen) {
  int charsPrinted;
  if (indexOf(settings->host, strlen(settings->host), ':', 0) != -1) {
    charsPrinted = snprintf(buffer, bufferLen, "https://[%s]:%d%s", settings->host, settings->port, settings->path);
  } else {
    charsPrinted = snprintf(buffer, bufferLen, "https://%s:%d%s", settings->host, settings->port, settings->path);
  }
  if (charsPrinted < 0) {
    memset(buffer, 0, bufferLen);
  }
  return buffer;
}

static int jwkTaskMain(RLETask *task) {
...
  if (success) {
    zowelog(NULL, LOG_COMP_ID_JWK, ZOWE_LOG_INFO, ZSS_LOG_JWK_READY_MSG, settings->fallback ? "with" : "without");
  } else {
    char url[256];
    zowelog(NULL, LOG_COMP_ID_JWK, ZOWE_LOG_WARNING, ZSS_LOG_JWK_FAILED_MSG, printJWKURL(settings, url, sizeof(url)));
  }
...
}

Copy link
Copy Markdown
Member Author

@1000TurquoisePogs 1000TurquoisePogs Nov 25, 2025

Choose a reason for hiding this comment

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

This looks to make the code more complicated. Why do this?

Copy link
Copy Markdown
Contributor

@ifakhrutdinov ifakhrutdinov Nov 27, 2025

Choose a reason for hiding this comment

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

Why do this?

Because it's not more complicated?

In the current implementation, you mix lower and higher level logic, and then you have to introduce additional message templates just to work around the peculiarities of IPv6; this all basically unnecessarily spreads lower level details over a larger area than needed.

So, why not just hide this in a specialised function which can be easily reviewed and verified?

zowelog(NULL, LOG_COMP_ID_JWK, ZOWE_LOG_WARNING, ZSS_LOG_JWK_FAILED_MSG);
if (indexOf(settings->host, strlen(settings->host), ':', 0) != -1) {
//wraps ipv6 address in []
zowelog(NULL, LOG_COMP_ID_JWK, ZOWE_LOG_WARNING, ZSS_LOG_JWK_FAILED_IPV6_MSG, settings->host, settings->port, settings->path);
} else {
zowelog(NULL, LOG_COMP_ID_JWK, ZOWE_LOG_WARNING, ZSS_LOG_JWK_FAILED_MSG, settings->host, settings->port, settings->path);
}
}
fflush(stdout);
}
Expand Down Expand Up @@ -280,6 +288,12 @@ static void getPublicKey(Json *jwk, x509_public_key_info *publicKeyOut, int *sta
if (!keyObject) {
zowelog(NULL, LOG_COMP_ID_JWK, ZOWE_LOG_WARNING, "JWK doesn't contain key\n");
*statusOut = JWK_STATUS_UNRECOGNIZED_FMT_ERROR;

zowelog(NULL, LOG_COMP_ID_JWK, ZOWE_LOG_WARNING, "JWK response:\n");
Copy link
Copy Markdown
Contributor

@Martin-Zeithaml Martin-Zeithaml Nov 20, 2025

Choose a reason for hiding this comment

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

Probably needs fflush(stdout); after the zowelog. Otherwise I can see message header after the text:

"possible error message"JWK response:

instead of

JWK response:
"possible error message"

Copy link
Copy Markdown
Contributor

@ifakhrutdinov ifakhrutdinov Nov 20, 2025

Choose a reason for hiding this comment

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

On top of that, the message might get split by some other stray message since they're not printed as a single message.

//Often enough, the destination has some error message that can be printed
jsonPrinter *jp = makeJsonPrinter(STDOUT_FILENO);
Comment thread
ifakhrutdinov marked this conversation as resolved.
jsonPrintObject(jp, jwkObject);
freeJsonPrinter(jp);
return;
}

Expand Down
4 changes: 3 additions & 1 deletion h/zssLogging.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,8 +518,10 @@ bool isLogLevelValid(int level);
#ifndef ZSS_LOG_JWK_FAILED_MSG_ID
#define ZSS_LOG_JWK_FAILED_MSG_ID ZSS_LOG_MSG_PRFX"1605W"
#endif
#define ZSS_LOG_JWK_FAILED_MSG_TEXT "Server will not accept JWT\n"
#define ZSS_LOG_JWK_FAILED_MSG_TEXT "Server will not accept JWT\nCheck URL https://%s:%d%s for errors.\n"
Copy link
Copy Markdown
Contributor

@ifakhrutdinov ifakhrutdinov Nov 20, 2025

Choose a reason for hiding this comment

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

I suggest we have a single messages: Server will not accept JWT\nCheck URL %s for errors.\n and format the path in a separate function. The reason for that is for a simple task (print an error message) we're spilling so much unnecessary details here and in jwkTaskMain; it's better to isolate that.

#define ZSS_LOG_JWK_FAILED_MSG ZSS_LOG_JWK_FAILED_MSG_ID" "ZSS_LOG_JWK_FAILED_MSG_TEXT
#define ZSS_LOG_JWK_FAILED_IPV6_MSG_TEXT "Server will not accept JWT\nCheck URL https://[%s]:%d%s for errors.\n"
#define ZSS_LOG_JWK_FAILED_IPV6_MSG ZSS_LOG_JWK_FAILED_MSG_ID" "ZSS_LOG_JWK_FAILED_MSG_TEXT

#ifndef ZSS_LOG_JWK_RETRY_MSG_ID
#define ZSS_LOG_JWK_RETRY_MSG_ID ZSS_LOG_MSG_PRFX"1606W"
Expand Down
Loading