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

Add HtmlProofer to validate links as well as fix current issues #1851

Merged
merged 2 commits into from Nov 7, 2019

Conversation

stevesloka
Copy link
Member

Signed-off-by: Steve Sloka slokas@vmware.com

@stevesloka stevesloka added this to the 1.1.0 milestone Nov 4, 2019
@stevesloka
Copy link
Member Author

@SDBrett

@stevesloka
Copy link
Member Author

I didn't realize but this might be similar to #1848

Copy link
Contributor

@jpeach jpeach left a comment

Choose a reason for hiding this comment

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

LGTM, I just had a few nits here and there.

Makefile Outdated
@@ -39,7 +39,7 @@ vet: | test
go vet $(MODULE)/...

check: ## Run tests and CI checks
check: test test-race vet gofmt staticcheck misspell unconvert unparam ineffassign yamllint check-stale
check: test test-race vet gofmt staticcheck misspell unconvert unparam ineffassign yamllint check-stale site-check
Copy link
Contributor

Choose a reason for hiding this comment

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

My workflow is normally make check, but the Docker install takes a long time. I can switch to make test but not sure whether we want check to take longer by default.

Copy link
Member Author

Choose a reason for hiding this comment

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

make check is what we run for each PR on Travis. Obviously, we can change that and run multiple things but my initial thought was to run this on each check. For now, I'm going to take out because there are other issues involved with this to run via CI.

Makefile Outdated
@@ -218,6 +218,10 @@ site-devel: ## Launch the website in a Docker container
docker run --publish $(JEKYLL_PORT):$(JEKYLL_PORT) -v $$(pwd)/site:/site -it $(JEKYLL_IMAGE) \
bash -c "cd /site && bundle install && bundle exec jekyll serve --host 0.0.0.0 --port $(JEKYLL_PORT) --livereload"

site-check: ## Launch the website in a Docker container
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please update the doc comment?

site/_config.yml Outdated
@@ -28,7 +28,7 @@ footer_social_links:
url: https://kubernetes.slack.com/messages/contour
RSS:
fa_icon: fa fa-rss
url: feed.xml
url: http://projectcontour.io/feed.xml
Copy link
Contributor

Choose a reason for hiding this comment

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

HTTPS?

site/_config.yml Outdated
@@ -46,6 +46,7 @@ defaults:
layout: "default"

repository: projectcontour/contour
github: [metadata]
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this do?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removes the errors when not providing GH creds. It was failing the Travis build.

@@ -36,7 +36,7 @@ Until Contour release v0.14, the deployment model placed Contour and Envoy in th
The split model allows Contour and Envoy to scale independently. If a new version of Contour is released, you can now upgrade to the new version without having to restart each instance of Envoy in your cluster.

A key new feature in Contour v0.14 is that we have secured the communication between Contour and Envoy over the xDS API connection utilizing mutually checked self-signed certificates. There are three ways to generate certificates to secure this connection.
The Contour repo includes step-by-step examples of how to generate certificates from a command line if you want to [generate them by hand](https://github.com/projectcontour/contour/blob/master/docs/grpc-tls-howto.md#generating-example-grpc-tls-certificates); the example/ds-hostnet-split [example](https://github.com/projectcontour/contour/tree/master/examples/ds-hostnet-split) includes a [job](https://github.com/projectcontour/contour/blob/master/examples/ds-hostnet-split/02-job-certgen.yaml) which automatically generate the certificates, or you could provide your own based on your IT security requirements.
The Contour repo includes step-by-step examples of how to generate certificates from a command line if you want to [generate them by hand](https://projectcontour.io/guides/grpc-tls-howto#generating-example-grpc-tls-certificates); the example/contour [example](https://github.com/projectcontour/contour/blob/master/examples/contour) includes a [job](https://github.com/projectcontour/contour/blob/master/examples/contour/02-job-certgen.yaml) which automatically generate the certificates, or you could provide your own based on your IT security requirements.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use site.github.repository_url for these links?

@@ -23,7 +23,7 @@ This page describes the compatibility matrix of Contour and Envoy versions.
## Envoy extensions

Contour requires the following extensions.
If you are using the image recommended in our [example deployment](https://github.com/projectcontour/contour/blog/{{ site.github.latest_release.tag_name }}/examples/contour) no action is required.
If you are using the image recommended in our [example deployment](https://github.com/projectcontour/contour/tree/{{ site.github.latest_release.tag_name }}/examples/contour) no action is required.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use site.github.repository_url here?


# TLS support

Contour 0.3 adds support for HTTPS (TLS/SSL) ingress by integrating Envoy's SNI support.
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't 0.3 ancient history? How about we just say that Contour supports TLS for Ingress and HTTPProxy?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, this PR wasn't looking to update previous docs, but I can do that here.

…site

Signed-off-by: Steve Sloka <slokas@vmware.com>
@SDBrett
Copy link
Contributor

SDBrett commented Nov 5, 2019

Over all it LGTM.

SDBrett added a commit to SDBrett/contour that referenced this pull request Nov 6, 2019
Adjustments to colour themes to improve contrast rations
Addition of titles to iframes for resources page
Updated buttons to improve contrast on highlight
Addition of link titles

Note: Image alt tags where not adjusted as they have been referenced in PR projectcontour#1851

Reference:
Google lighthouse fundamentals: https://developers.google.com/web/fundamentals/accessibility?utm_source=lighthouse&utm_medium=devtools

Site accessibility rules: https://dequeuniversity.com/rules/axe/3.4

Signed-off-by: Brett Johnson <brett@sdbrett.com>
@stevesloka stevesloka merged commit 2006d9d into projectcontour:master Nov 7, 2019
@stevesloka stevesloka deleted the addHtmlProofer branch November 7, 2019 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants