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

upgrade go to 1.16 and add SAN to all certs #2164

Closed
wants to merge 1 commit into from
Closed

upgrade go to 1.16 and add SAN to all certs #2164

wants to merge 1 commit into from

Conversation

cindy52
Copy link
Contributor

@cindy52 cindy52 commented Jun 30, 2021

What type of PR is this?

Uncomment only one /kind <> line, press enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking
/kind bug

kind cleanup

/kind documentation
/kind feature
/kind hotfix

What this PR does / Why we need it:
Upgrade go version to the latest (1.16.5). Go 1.15 no longer supports Common Name based certs for HTTPS without a specific flag. This PR includes adding SAN for all related certs and updating the doc to create a cert with SAN for allocation service.

Which issue(s) this PR fixes:

Closes #1899
Closes #2024

Special notes for your reviewer:

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cindy52
To complete the pull request process, please assign markmandel after the PR has been reviewed.
You can assign the PR to them by writing /assign @markmandel in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 4b8add07-128a-4281-9ba5-63be47798dfd

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: ac121d3f-3218-4904-b111-1dc2c272131d

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@markmandel
Copy link
Member

In case you didn't see it:

docker run -e FILE="/go/src/agones.dev/agones/site/content/en/docs/Reference/agones_crd_api_reference.html" -e VERSION=1.16.0 --rm -i -v /workspace/build//.config/gcloud:/root/.config/gcloud -v ~/.kube/:/root/.kube -v ~/.config/helm:/root/.config/helm -v ~/.cache/helm:/root/.cache/helm -v /workspace:/go/src/agones.dev/agones -v /workspace/build//.gomod:/go/pkg/mod -v /workspace/build//.gocache:/root/.cache/go-build agones-build:d2646eda2f bash -c "/go/src/agones.dev/agones/site/gen-api-docs.sh"
using go proxy as a workaround for git.agache.org being down: http://proxy.golang.org
includes/website.mk:100: recipe for target 'test-gen-api-docs' failed
make[1]: Leaving directory '/workspace/build'

That seems weird actually, I'm not sure what is going on there. Can you repro locally?

Usually that means need to run gen-api-docs, but I don't see a diff issue there 🤔

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 3f0d8eef-b354-4133-afcd-af4424753e61

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@cindy52
Copy link
Contributor Author

cindy52 commented Jun 30, 2021

In case you didn't see it:

docker run -e FILE="/go/src/agones.dev/agones/site/content/en/docs/Reference/agones_crd_api_reference.html" -e VERSION=1.16.0 --rm -i -v /workspace/build//.config/gcloud:/root/.config/gcloud -v ~/.kube/:/root/.kube -v ~/.config/helm:/root/.config/helm -v ~/.cache/helm:/root/.cache/helm -v /workspace:/go/src/agones.dev/agones -v /workspace/build//.gomod:/go/pkg/mod -v /workspace/build//.gocache:/root/.cache/go-build agones-build:d2646eda2f bash -c "/go/src/agones.dev/agones/site/gen-api-docs.sh"
using go proxy as a workaround for git.agache.org being down: http://proxy.golang.org
includes/website.mk:100: recipe for target 'test-gen-api-docs' failed
make[1]: Leaving directory '/workspace/build'

That seems weird actually, I'm not sure what is going on there. Can you repro locally?

Usually that means need to run gen-api-docs, but I don't see a diff issue there 🤔

It's weird. Every time it fails with different errors. Last time I checked it failed with build-agones-sdk-image-windows....

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 574b18ab-e747-42ba-9a79-32d3fde4a737

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 0d083f87-d263-4d52-a4ae-5cceee0d1851

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: f9c47a63-572f-4398-a422-8f156df8c66e

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@cindy52
Copy link
Contributor Author

cindy52 commented Jul 1, 2021

gen-api-docs currently failed because In Go 1.16, module-aware commands report an error after discovering a problem in go.mod or go.sum instead of attempting to fix the problem automatically. The related issue can be found here golang/go#44529. I tried to fix it as suggested by changing the replacement and add -mod=mod with go build in gen-api-docs.sh file. The build was successfully after that but I'm getting an error type invalid type has kind=Unsupported which is unhandled when generating the api doc. Found some related issues here kubeflow/website#1899. Looks like there is not a known fix yet.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 99e980ce-b586-48cb-b346-ecd224b2b136

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@@ -4,7 +4,7 @@ echo "Generating certs..."
echo "Email should be: [email protected]"
echo "Common Name should be: agones-controller-service.agones-system.svc"
openssl genrsa -out server.key 2048
openssl req -new -x509 -sha256 -key server.key -out server.crt -days 3650
openssl req -new -x509 -sha256 -key server.key -out server.crt -days 3650 -addext 'subjectAltName=DNS:agones-controller-service.agones-system.svc'
Copy link
Member

Choose a reason for hiding this comment

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

Aaah. That's how you do it!

@markmandel
Copy link
Member

Total shot in the dark, but this is the library we use:
https://github.com/ahmetb/gen-crd-api-reference-docs

I see a similar issue here: ahmetb/gen-crd-api-reference-docs#10

Also wondering if an upgrade helps? 🤷🏻

@cindy52
Copy link
Contributor Author

cindy52 commented Jul 1, 2021

Total shot in the dark, but this is the library we use:
https://github.com/ahmetb/gen-crd-api-reference-docs

I see a similar issue here: ahmetb/gen-crd-api-reference-docs#10

Also wondering if an upgrade helps? 🤷🏻

I tried upgrading to the latest but still got the same err..

@markmandel
Copy link
Member

Digging into how other people fixed this on their end, looks like they found a solution here: https://github.com/knative/docs/pull/2054/files

But the author does work here at Google (he's on my team 😄 ) we could always go bug him and see what he knows.

It makes me feel like we're missing a reference to something somewhere.

@cindy52
Copy link
Contributor Author

cindy52 commented Jul 1, 2021

Digging into how other people fixed this on their end, looks like they found a solution here: https://github.com/knative/docs/pull/2054/files

But the author does work here at Google (he's on my team 😄 ) we could always go bug him and see what he knows.

It makes me feel like we're missing a reference to something somewhere.

I saw it yesterday and even looks into his error logs in the issue: ahmetb/gen-crd-api-reference-docs#10. I believe the cause is different. For knative issue, it turns out some of the import paths are github.com instead of knative.dev, it shows in the error log he attached. I turned on detailed log as well and all import paths are agones.dev. It shows in the details of the failed build.

@roberthbailey roberthbailey added the feature-freeze-do-not-merge Only eligible to be merged once we are out of feature freeze (next full release) label Jul 13, 2021
@roberthbailey roberthbailey removed the feature-freeze-do-not-merge Only eligible to be merged once we are out of feature freeze (next full release) label Jul 20, 2021
@markmandel
Copy link
Member

Should we close this PR for now?

@cindy52 cindy52 closed this Jul 28, 2021
@cindy52
Copy link
Contributor Author

cindy52 commented Jul 28, 2021

Should we close this PR for now?

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't run Allocation example with Go 1.16 Migrate to using SANs for webhook certificates for Go 1.15
5 participants