change test to match current message when decryption fails

[LeeDr]

Summary

Apparently the message changed when testing an action without the correct encryption key. I'll have Kibana-alerting team review to make sure this was intentional.

For the test email connectors, without the key used to create it, 'should show a failure callout' in
kibana/x-pack/test/stack_functional_integration/apps/alerts/alerts_encryption_keys.js

Change from:
'Test failed to run\nThe following error was found:\nerror sending email\nDetails:\nMail command failed: 550 5.7.1 Relaying denied'

to:
'Test failed to run\nThe following error was found:\nCannot read properties of undefined (reading \'password\')'

[elasticmachine]

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

[pmuellr]

I'm a little confused. Is this breaking CI, some previous merge changed something? Or is this test not run in CI?

The change itself does not look good to me. The "from" version clearly has some kind of a message from a mail server in it - I guess this is testing the email connector. The "to" version looks like an error occurred in the alerting framework while it was executing the connector.

So it looks like a bug got injected into alerting ... somehow, sometime.

When was the last time this was run? How can I run it?

[LeeDr]

You're correct in that it doesn't run as part of CI. It runs with the integration-test repo. I think it would run OK in CI if it had just the right kibana.yml with the right encryption keys.
In this test there are 3 email actions that get tested. One with the current encryption key, one with a decrypt only key, and one where we don't have the right key. It's this last one where the message changed.

The test is still passing on 7.17.

Unfortunately I can't tell when it started failing on main branch because other issues were causing the whole job to fail.

[ymao1]

It looks to me like this test caught a bug, possible with a changes in format to the email connector that we've introduced to handle OAuth. It looks like the connectors loaded from esarchive as part of this test are from 7.11 and possibly we're missing some sort of migration? I will investigate further

[ymao1]

I believe this is because of this PR which changes the way decryption errors are handled during action migrations. Previously, if an decryption error occurred during migration, the migration would not be applied to the action saved object (and no error thrown during migration) so the resulting saved object would have a malformed schema. With this PR, in the event of decryption errors, the migration is applied to the un-decrypted saved object and decrypted attributes are stripped. This functional test is testing this exact scenario, where the incorrect encryption key will cause decryption failures on migration, so the decrypted secrets attribute is stripped from the saved object and the following is logged:

[2022-01-05T10:20:06.186-05:00][ERROR][plugins.encryptedSavedObjects] Failed to decrypt "secrets" attribute: Unsupported state or unable to authenticate data
[2022-01-05T10:20:06.187-05:00][WARN ][savedobjects-service] Decryption failed for encrypted Saved Object "7b563ad0-6e3a-11ec-bb96-95d68d7be48c" of type "action" with error: Unable to decrypt attribute "secrets". Encrypted attributes have been stripped from the original document and migration will be applied but this may cause errors later on.

Because secrets is stripped from the saved object, the expected object that the username/password are stored in is missing, leading to the Cannot read properties of undefined error.

I am a little confused by why this would pass in 7.17 though because this PR was merged for 7.15 and there are migrations for 7.16

[ymao1]

Created a new issue for this: #122346 @pmuellr WDYT?

[pmuellr]

Thanks for digging Ying, this does in fact sound like what's happening.

I'm guessing this set of tests is not actually run with every CI PR build. Maybe we haven't run it since before the change?

[ymao1]

Interesting. I just migrated to 7.17 and tested it and did not see the Cannot read properties of undefined error, just an error that looks like it came from the mail provider.

Looks like it was adding this additional connector validation that changed things: #116079

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: bdede96
• Storybooks Preview

Metrics [docs]

✅ unchanged

History

• 💔 Build #19234 failed 711d233
• 💛 Build #15728 was flaky aa9d0bb
• 💔 Build #15574 failed 808482b

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[ymao1]

LGTM!

[kibanamachine]

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 122224 or prevent reminders by adding the backport:skip label.