-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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.
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
Update security deprecation messages #115241
Changes from 3 commits
3913a50
ace4b2c
e584ec4
2c66a36
b409027
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,56 +61,13 @@ export const deprecations = ({ | |
} | ||
return config; | ||
}, | ||
(config, fromPath, addDeprecation) => { | ||
const es: Record<string, any> = get(config, 'elasticsearch'); | ||
if (es) { | ||
if (es.username === 'elastic') { | ||
addDeprecation({ | ||
configPath: 'elasticsearch.username', | ||
message: `Setting [${fromPath}.username] to "elastic" is deprecated. You should use the "kibana_system" user instead.`, | ||
correctiveActions: { | ||
manualSteps: [`Replace [${fromPath}.username] from "elastic" to "kibana_system".`], | ||
}, | ||
}); | ||
} else if (es.username === 'kibana') { | ||
addDeprecation({ | ||
configPath: 'elasticsearch.username', | ||
message: `Setting [${fromPath}.username] to "kibana" is deprecated. You should use the "kibana_system" user instead.`, | ||
correctiveActions: { | ||
manualSteps: [`Replace [${fromPath}.username] from "kibana" to "kibana_system".`], | ||
}, | ||
}); | ||
} | ||
} | ||
return config; | ||
}, | ||
(config, fromPath, addDeprecation) => { | ||
const ssl: Record<string, any> = get(config, 'elasticsearch.ssl'); | ||
if (ssl) { | ||
if (ssl.key !== undefined && ssl.certificate === undefined) { | ||
addDeprecation({ | ||
configPath: 'elasticsearch.ssl.key', | ||
message: `Setting [${fromPath}.key] without [${fromPath}.certificate] is deprecated. This has no effect, you should use both settings to enable TLS client authentication to Elasticsearch.`, | ||
correctiveActions: { | ||
manualSteps: [ | ||
`Set [${fromPath}.ssl.certificate] in your kibana configs to enable TLS client authentication to Elasticsearch.`, | ||
], | ||
}, | ||
}); | ||
} else if (ssl.certificate !== undefined && ssl.key === undefined) { | ||
addDeprecation({ | ||
configPath: 'elasticsearch.ssl.certificate', | ||
message: `Setting [${fromPath}.certificate] without [${fromPath}.key] is deprecated. This has no effect, you should use both settings to enable TLS client authentication to Elasticsearch.`, | ||
correctiveActions: { | ||
manualSteps: [ | ||
`Set [${fromPath}.ssl.key] in your kibana configs to enable TLS client authentication to Elasticsearch.`, | ||
], | ||
}, | ||
}); | ||
} | ||
} | ||
return config; | ||
}, | ||
rename('xpack_api_polling_frequency_millis', 'licensing.api_polling_frequency'), | ||
|
||
// TODO: Add deprecations for "monitoring.ui.elasticsearch.username: elastic" and "monitoring.ui.elasticsearch.username: kibana". | ||
// TODO: Add deprecations for using "monitoring.ui.elasticsearch.ssl.certificate" without "monitoring.ui.elasticsearch.ssl.key", and | ||
// vice versa. | ||
// ^ These deprecations should only be shown if they are explicitly configured for monitoring -- we should not show Monitoring | ||
// deprecations for these settings if they are inherited from the Core elasticsearch settings. | ||
// See the Core implementation: src/core/server/elasticsearch/elasticsearch_config.ts | ||
Comment on lines
+64
to
+69
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The monitoring code had identical deprecations for (1), (2), (3), and (4) in the PR description. It would basically make those deprecations show up twice, which is confusing. We currently don't have the ability to differentiate if these options were set in the raw Monitoring config ( Since these are not breaking in 8.0, I think we should remove these Monitoring deprecations completely until we refactor the code and can differentiate between the two. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Is there a follow up issue for that? Removing deprecations that seem to be duplicates make sense. I just want to make sure we do actually follow up on this a.s.a.p. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll make a note to create a follow-up after this merges. I'll add a permalink to this section in the follow up issue's description. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a follow-up issue for this: #115597 |
||
]; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed Core deprecations here, but these particular ones were added by the Platform Security team, so I thought it would be prudent to update them along with our other deprecations 👍