Skip to content
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

Kibana link checking is broken #1805

Closed
bmorelli25 opened this issue Apr 10, 2020 · 5 comments
Closed

Kibana link checking is broken #1805

bmorelli25 opened this issue Apr 10, 2020 · 5 comments
Assignees
Labels
bug Something that does not look or behave correctly link-checking Link Checking & Redirects

Comments

@bmorelli25
Copy link
Member

bmorelli25 commented Apr 10, 2020

The Kibana link checker doesn't seem to be catching broken links.

docs/build_docs.pl

Lines 334 to 399 in 0cee4e4

sub check_kibana_links {
#===================================
my $build_dir = shift;
my $link_checker = shift;
my $branch;
say "Checking Kibana links";
# ${baseUrl}guide/en/elasticsearch/reference/${urlVersion}/modules-scripting-expression.html
# ${ELASTIC_WEBSITE_URL}guide/en/beats/filebeat/${DOC_LINK_VERSION}
# ${ELASTIC_DOCS}search-aggregations-bucket-datehistogram-aggregation.html
my $extractor = sub {
my $contents = shift;
return sub {
while ( $contents =~ m!`(\$\{(?:baseUrl|ELASTIC_.+)\}[^`]+)`!g ) {
my $path = $1;
$path =~ s/\$\{(?:DOC_LINK_VERSION|urlVersion)\}/$branch/;
$path
=~ s!\$\{ELASTIC_DOCS\}!en/elasticsearch/reference/$branch/!
|| $path =~ s!\$\{(?:baseUrl|ELASTIC_WEBSITE_URL)\}guide/!!;
return ( split /#/, $path );
}
return;
};
};
my $src_path = 'src/ui/public/documentation_links/documentation_links';
my $legacy_path = 'src/legacy/ui/public/documentation_links/documentation_links';
my $repo = ES::Repo->get_repo('kibana');
my @branches = sort map { $_->basename }
grep { $_->is_dir } $build_dir->subdir('en/kibana')->children;
my $link_check_name = 'link-check-kibana';
for (@branches) {
$branch = $_;
next if $branch eq 'current' || $branch =~ /^\d/ && $branch lt 5;
say " Branch $branch";
my $links_file;
my $source = eval {
$links_file = $src_path . ".js";
$repo->show_file( $link_check_name, $branch, $links_file );
} || eval {
$links_file = $src_path . ".ts";
$repo->show_file( $link_check_name, $branch, $links_file );
} || eval {
$links_file = $legacy_path . ".js";
$repo->show_file( $link_check_name, $branch, $links_file );
} || eval {
$links_file = $legacy_path . ".ts";
$repo->show_file( $link_check_name, $branch, $links_file );
};
die "failed to find kibana links file;\n$@" unless $source;
$link_checker->check_source( $source, $extractor,
"Kibana [$branch]: $links_file" );
# Mark the file that we need for the link check done so we can use
# --keep_hash with it during some other build.
$repo->mark_done( $link_check_name, $branch, $links_file, 0 );
}
}

Here's an example of a broken link: elastic/kibana#63295.

@bmorelli25 bmorelli25 added the bug Something that does not look or behave correctly label Apr 10, 2020
@gtback gtback self-assigned this Apr 15, 2020
@gtback
Copy link
Member

gtback commented Apr 16, 2020

I'm only starting to understand how Kibana link checking works. @nik9000 gave me an overview this morning. It might take me a little while to wrap my head around it all.

@gtback gtback added the link-checking Link Checking & Redirects label May 14, 2020
@gtback gtback added defer and removed defer labels Sep 4, 2020
gtback added a commit to gtback/kibana that referenced this issue Jan 7, 2021
This link is currently wrong, but I'm testing to make sure it will get
caught if it's included in this file. A future commit will actually use
this variables in vega_help_menu.tsx, and update the link in
`doc_links_service.ts` to be correct.

This is part of a fix for elastic#87367
Related: elastic/docs#1805
@gtback
Copy link
Member

gtback commented Jan 15, 2021

I figured out why this was broken (at least for elastic/kibana#63295):

The only links that are checked are those that are located in the root Doc Links file (which has moved and been renamed between versions of Kibana)

If you do something like this in a different file, and then build URLs in those files, those URLs won't be checked.

import { ELASTIC_WEBSITE_URL, DOC_LINK_VERSION } from 'ui/documentation_links';

This is what @lcawl is trying to fix in elastic/kibana#88107.

@gtback
Copy link
Member

gtback commented Jan 15, 2021

I'm tempted to close this, because I don't think looking through all of the Kibana code for links is feasible, but I'd like to track down why elastic/kibana#87721 wasn't failing as well.

@gtback
Copy link
Member

gtback commented Mar 16, 2021

I believe elastic/kibana#94274 (which prevents the doc_links_service.ts from being checked) is the ultimate root cause of this (along with elastic/kibana#88107: some of the links aren't in that file so aren't being checked).

Order of operations for resolving this:

@gtback
Copy link
Member

gtback commented Mar 22, 2021

elastic/kibana#87721 is passing! 🙌🏻 So I'm confident we have this fixed. The actual link will get fixed once that's merged, but that's not a blocker here.

@gtback gtback closed this as completed Mar 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that does not look or behave correctly link-checking Link Checking & Redirects
Projects
None yet
Development

No branches or pull requests

2 participants