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

Use appropriate reserved domain for example URLs #910

Merged
merged 9 commits into from
Aug 12, 2020

Conversation

ebeahan
Copy link
Member

@ebeahan ebeahan commented Aug 6, 2020

Summary

Two minor documentation changes for the event.* field set:

  • Removed the "live" URL links by adding quotes to the string. This is actually incorrect. Our documentation generator is handling this by always wrapping example strings with backticks (`). As long as we ensure the examples don't use potentially valid domains, I don't think we need to worry about being "defensive" for other doc locations which may render a hyperlink for the URLs (e.g. Beats).

  • Switched example URLs to use the IANA reserved *.example.com

@ebeahan ebeahan self-assigned this Aug 6, 2020
webmat
webmat previously approved these changes Aug 6, 2020
Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

LGTM

Have you looked at some of the other places where they may show up, like url and source/dest/client/server?

@ebeahan
Copy link
Member Author

ebeahan commented Aug 6, 2020

I noted the other places using real domains:

Field(s) Example Value
*.registered_domain www.google.com
package.reference https://golang.org
tls.client.server_name, url.domain www.elastic.co
x509.issuer.organizational_unit www.digicert.com
x509.subject.common_name r2.shared.global.fastly.net
x509.subject.distinguished_name C=US, ST=California, L=San Francisco, O=Fastly, Inc., CN=r2.shared.global.fastly.net

While it's top of mind, should we unify all of our example usages to example.com for uniformity? ♻️

generated artifacts
@webmat
Copy link
Contributor

webmat commented Aug 6, 2020

Yeah if you're up for it 👍

I guess we may have to make a few adjustments outside of these domains per se, for the examples to continue making sense. Example for x509.subject.distinguished_name:

C=US, ST=California, L=San Francisco, O=Example, Inc., CN=certs.example.com

Out of the ones you've outlined, I think I would leave package.reference at "https://golang.org". I think this one is important at conveying that we're not looking for a download link, but more of a documentation page or the home of whatever software is in the package.

@webmat
Copy link
Contributor

webmat commented Aug 6, 2020

Whenever you work on some of these domains, make sure to adjust the domain breakdowns as well. www.example.com => .registered_domain: example.com, .top_level_domain: com

@webmat webmat added the ready Issues we'd like to address in the future. label Aug 11, 2020
@ebeahan
Copy link
Member Author

ebeahan commented Aug 11, 2020

@webmat this is ready for another 👀

Should we plan on backporting these changes to 1.5?

Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

I think the only remaining domain we could adjust is on server.subject, where we have "www.mydomain.com". Could you adjust it as well in the same way?

Yeah I think it makes sense to backport this to 1.5. Note that if Beats wants to reimport our field defs to adjust some of the examples that come from ECS, we'd have to cut 1.5.1. But unless this is clearly needed, I think we can simply backport to 1.5 for now, and at least have the main docs corrected.

Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

LGTM

@ebeahan ebeahan merged commit c114f23 into elastic:master Aug 12, 2020
@ebeahan ebeahan deleted the fix-url branch August 12, 2020 18:20
ebeahan added a commit to ebeahan/ecs that referenced this pull request Aug 12, 2020
# Conflicts:
#	CHANGELOG.next.md
#	docs/field-details.asciidoc
#	generated/beats/fields.ecs.yml
#	generated/csv/fields.csv
#	generated/ecs/ecs_flat.yml
#	generated/ecs/ecs_nested.yml
#	schemas/event.yml
#	schemas/x509.yml
ebeahan added a commit to ebeahan/ecs that referenced this pull request Aug 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.5.0 1.6.0 documentation ready Issues we'd like to address in the future.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants