dev_docs/contributing/best_practices.mdx
Be sure to follow our <DocLink id="kibDevPrinciples" text="development principles"/> and <DocLink id="kibStandards" text="standards and guidelines"/>.
Documentation best practices can be found <DocLink id="kibDocumentation" text="here"/>.
Build with scalability in mind.
Did you know Kibana makes a public statement about our commitment to creating an accessible product for people with disabilities? We do! It's very important all of our apps are accessible.
Kibana maintains accessibility quality using a layered, hybrid approach:
@elastic/eslint-plugin-eui – custom rules for the ESLint libraryCI tooling: running axe-core check in FTR, Scout and Cypress tests.
Enforces: Semantics, landmarks, contrast, focus order, interaction patterns.
Note: All tests are based on the common configuration defined in src/platform/packages/shared/kbn-axe-config/index.ts.
Usage:
FTR a11y tests (tests must be placed in the test/accessibility folder):
const a11y = getService('a11y');
...
it('has no detectable a11y violations on load', async () => {
await common.navigateToApp('dashboard');
await a11y.testAppSnapshot();
});
Scout using page.checkA11y. Run an accessibility check on a specific root element:
test('has no detectable a11y violations', async ({ page }) => {
const { violations } = await page.checkA11y({ include: ['{CSS selector of root element you are testing}'] });
expect(violations).toHaveLength(0);
});
Cypress using cy.checkA11y. A custom Cypress command is available to run accessibility checks:
it('has no detectable a11y violations on load', () => {
cy.checkA11y();
});
Action: Improve coverage — add or extend tests while developing features.
For help: join #accessibility slack channel.
</DocCallOut>
Kibana is translated into other languages. Use our i18n utilities to ensure your public facing strings will be translated to ensure all Kibana apps are localized. Read and adhere to our i18n guidelines
<DocCallOut title="Internal only"> Elasticians, check out the #kibana-localization channel to ask questions and receive guidance. </DocCallOut>We use es-lint rules when possible, but please review our styleguide, which includes recommendations that can't be linted on.
Es-lint overrides on a per-plugin level are discouraged.
The SavedObjectClient should always be used for reading and writing saved objects that you manage. For saved objects managed by other plugins, their plugin APIs should be used instead.
Good:
const dataView = dataViewStartContract.get(dataViewId);
Bad:
const dataView = savedObjectsClient.get(dataViewId) as DataView;
Use EUI for all your basic UI components to create a consistent UI experience. We also have generic UI components offered from the <DocLink id="kibKibanaReactPluginApi" text="kibana_react" /> plugin.
Don't export <DocLink id="kibPlatformIntro" section="public-plugin-api" text="public APIs"/> without reason. Make your public APIs as small as possible. You will have to maintain them, and consider backward compatibility when making changes.
Over-refactoring can be a problem in it's own right, but it's still important to be aware of the existing services that are out there and use them when it makes sense. Check out our <DocLink id="kibBuildingBlocks" text="building blocks"/> to see what high-level services are at your disposal. In addition, our <DocLink id="kibDevDocsApiWelcome" text="API documentation" /> lists additional services.
Try not to put your PR in review mode, or merge large changes, right before Feature Freeze. It's inevitably one of the most volatile times for the Kibana code base, try not to contribute to this volatility. Doing this can:
All of the above contributes to more bugs being found in the QA cycle and can cause a delay in the release. Prefer instead to merge your large change right after feature freeze. If you are worried about missing your initial release version goals, review our <DocLink id="devTimeBasedReleases" text="release train philosophy"/>. It's okay!
</DocCallOut>When possible, build features with incremental sets of small and focused PRs, but don't check in unused code, and don't expose any feature on main that you would not be comfortable releasing.
If your feature cannot be broken down into smaller components, or multiple engineers will be contributing, you have a few other options to consider.
1. Hide your feature behind a feature flag
Features can be merged behind a flag if you are not ready to make them the default experience, but all code should still be tested, complete and bug free.
A good question to ask yourself is, how will you feel if a customer turns this feature on? Is it usable, even if not up to the level of something we would market? It should have some level of minimal utility.
Another question to ask yourself is, if this feature gets cancelled, how difficult will it be to remove?
2. Develop on a feature branch
This option is useful if you have more than one contributor working on a large feature. The downside is handling code conflicts when rebasing with the main branch.
Consider how you want to handle the PR review. Reviewing each PR going into the feature branch can lighten the review process when merging into the main branch.
3. Use an example plugin
If you are building a service for developers, create an example plugin to showcase and test intended usage. This is a great way for reviewers and PMs to play around with a feature through the UI, before the production UI is ready. This can also help developers consuming your services get hands on.
Kibana uses a monorepo and our processes and tooling are built around this decision. Utilizing a monorepo allows us to have a consistent peer review process and enforce the same code quality standards across all of Kibana's code. It also eliminates the necessity to have separate versioning strategies and constantly coordinate changes across repos.
When experimenting with code, it's completely fine to create a separate GitHub repo to use for initial development. Once the code has matured to a point where it's ready to be used within Kibana, it should be integrated into the Kibana GitHub repo.
There are some exceptions where a separate repo makes sense. However, they are exceptions to the rule. A separate repo has proven beneficial when there's a dedicated team collaborating on a package which has multiple consumers, for example EUI.
It may be tempting to get caught up in the dream of writing the next package which is published to npm and downloaded millions of times a week. Knowing the quality of developers that are working on Kibana, this is a real possibility. However, knowing which packages will see mass adoption is impossible to predict. Instead of jumping directly to writing code in a separate repo and accepting all the complications that come along with it, prefer keeping code inside the Kibana repo. A Kibana package follows the npm idioms and can be later converted into a npm package, moved into an external repo and be published into the npm if a good reason for it was found (for example to enable external contributions).
We are currently working to fully leverage change-based tasks across the repository and this functionality works best when the dependency tree is made of small packages focused around a single responsibility. Having said that, we do not think that contributors should break up their packages into single functions from day one; having 10k packages that contain a single function would also be a problem. Instead, breaking up packages over time will be a common practice used to address bottlenecks in the dependency tree and will be a valuable tool if build performance is being compromised.
The goal when designing your package should be to mainly ensure that it has a single responsibility, and that anyone depending on your package would reasonably need everything exposed by it. This is important for bundle sizes (the entire package will be loaded every time it is imported) and for change-based tasks (any time this package changes all dependent tasks will need to be run). Packages should be broken into smaller packages if they have multiple responsibilities or serve as a collection of related things which usually won't be necessary at the same time.
The "responsibilities" of a package are defined by the features, utilities, and components exposed by your package. If a developer tries to use your package, is it likely that they will use every part of it or just some of it? If most people would only need part of your package, then we would argue that your package has multiple responsibilities. At this point it is probably time to break up your package.
When making a change like this, import statements that previously pointed to the now broken-up package need to be updated to point to the smaller packages, so we have written an ESLint rule
called @kbn/imports/exports_moved_packages which allows contributors to define the exports previously available from one package as now being available from another package and leverage
auto-fixing to migration all existing and new uses of this export to the proper package.
_search queries.debug/trace level).Has there been a discussion about which license this feature should be available under? Open up a license issue in https://github.com/elastic/dev if you are unsure.
</DocCallOut>Every PR submitted should be accompanied by tests. Read through the <DocLink id="kibDevTutorialTestingPlugins" text="testing plugins tutorial" /> for how to test.
Refer to the list of browsers and OS Kibana supports https://www.elastic.co/support/matrix
Does the feature work efficiently on the below listed browsers
Any time you change state that is part of a Saved Object you will have to write a <DocLink id="kibDevDocsSavedObjectsIntro" section="migrations-and-backward-compatibility" text="migration" />.
Never store state from another plugin in your Saved Objects or URLs unless it implements the <DocLink id="kibDevDocsPersistableStateIntro" text="persistable state interface"/>. Remember to check for migrations when deserializing that state.
If you expose state and you wish to allow other plugins to persist you must ensure it implements the <DocLink id="kibDevDocsPersistableStateIntro" text="persistable state interface"/>. This is very common for by value entities, like visualizations that exist on a dashboard but are not part of the visualization library. If you make a breaking change to this state you must remember to register a migration for it.
Saved objects exported from past Kibana versions should always continue to work. Bookmarked URLs should also always work. Check out <DocLink id="kibDevKeyConceptsNavigation" section="specifying-state" text="URL Locators" /> to learn about migrating state in URLs.
Plugins should rarely, if ever, access Kibana's filesystem directly. Kibana instances are commonly ephemeral and anything written to the filesystem will potentially not be there on restart.
There are generally multiple instances of Kibana all hosted behind a round-robin load-balancer. As a result, storing state in server memory is risky as there is no guarantee that a single end-user's HTTP requests will be served by the same Kibana instance.
Kibana has a number of platform services that don't work with WebSockets, for example authentication and authorization. If your use-case would benefit substantially from websockets, talk to the Kibana Core team about adding support. Do not hack around this limitation, everytime that someone has, it's created so many problems it's been eventually removed.
When writing code for Kibana, be sure to follow these best practices to avoid common vulnerabilities. Refer to the included Open Web Application Security Project (OWASP) references to learn more about these types of attacks.
XSS is a class of attacks where malicious scripts are injected into vulnerable websites. Kibana defends against this by using the React
framework to safely encode data that is rendered in pages, the EUI framework to automatically sanitize
links, and a restrictive Content-Security-Policy header.
Best practices
dangerouslySetInnerHtml.Element.innerHTML and Element.outerHTML.href links. Otherwise, sanitize user input before rendering links to
ensure that they do not use the javascript: protocol.eval, Function, and _.template functions -- these are restricted by ESLint rules.setTimeout and setInterval in client-side code. If an attacker can manipulate the arguments and pass a string to
one of these, it is evaluated dynamically, which is equivalent to the dangerous eval function.CSRF is a class of attacks where a user is forced to execute an action on a vulnerable website that they're logged into, usually without their knowledge. Kibana defends against this by requiring custom request headers for API endpoints. For more information, see API Request Headers.
Best practices
OWASP reference for Command Injection, OWASP reference for Code Injection
RCE is a class of attacks where an attacker executes malicious code or commands on a vulnerable server. Kibana defends against this by using ESLint rules to restrict vulnerable functions, and by hooking into or hardening usage of these in third-party dependencies.
Best practices
eval, Function, and _.template functions -- these are restricted by ESLint rules.require.Prototype Pollution is an attack that is unique to JavaScript environments. Attackers can abuse JavaScript's prototype inheritance to
"pollute" objects in the application, which is often used as a vector for XSS or RCE vulnerabilities. Kibana defends against this by
hardening sensitive functions (such as those exposed by child_process), and by requiring validation on all HTTP routes by default.
Best practices
anObject[a][b] = c where a, b, and c are controlled by user input. This includes code paths where the
following logical code steps could be performed in separate files by completely different operations, or by recursively using dynamic
operations.__proto__ and prototype.constructor from being used within object
keys.Object.hasOwnProperty has arguments to the APIs that originate from an Object. An example is the defunct Code app's
spawnProcess
function.
child_process.spawn, child_process.exec, eval, Function('some string'), vm.runInContext(x),
vm.runInNewContext(x), vm.runInThisContext()eval, Function('some string'), setTimeout('some string', num), setInterval('some string', num)See also:
SSRF is a class of attacks where a vulnerable server is forced to make an unintended request, usually to an HTTP API. This is often used as a vector for information disclosure or injection attacks.
Best practices
kibana.yml, so only server administrators can change it.
transport.request with the Elasticsearch client, as no automatic escaping is performed.OWASP reference for Reverse Tabnabbing
Reverse tabnabbing is an attack where a link to a malicious page is used to rewrite a vulnerable parent page. This is often used as a vector
for phishing attacks. Kibana defends against this by using the EUI framework, which automatically adds the rel attribute to anchor tags,
buttons, and other vulnerable DOM elements.
Best practices
href attribute also have the
rel="noreferrer noopener" attribute specified. For more information, refer to the OWASP HTML5 Security Cheat
Sheet.Information disclosure is not an attack, but it describes whenever sensitive information is accidentally revealed. This can be configuration info, stack traces, or other data that the user is not authorized to access. This concern cannot be addressed with a single security control, but at a high level, Kibana relies on the hapi framework to automatically redact stack traces and detailed error messages in HTTP 5xx response payloads.
Best practices