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

Update go-tpm version to v0.3.2 that supports arm64 #276

Merged
merged 1 commit into from
Sep 23, 2021

Conversation

vinayakankugoyal
Copy link
Contributor

@vinayakankugoyal vinayakankugoyal commented Sep 21, 2021

Version 0.2.0 of go-tpm library did not support building for linux_arm64. See error below.

bazel build --platforms=@io_bazel_rules_go//go/toolchain:linux_arm64 //cmd/gke-exec-auth-plugin:gke-exec-auth-plugin 

workspace/vendor/github.com/google/go-tpm/tpmutil/poll_unix.go:28:33: undefined: syscall.SYS_POLL
Target //cmd/gke-exec-auth-plugin:gke-exec-auth-plugin failed to build
INFO: Elapsed time: 0.922s, Critical Path: 0.25s
INFO: 15 processes: 10 internal, 5 linux-sandbox.
FAILED: Build did NOT complete successfully

The failure above happens because v0.2.0 of the go-tpm library used syscall.SYS_POLL which is not supported in arm64 architecture. To workaround this golang has implemented a shim for poll on arm64 in https://go-review.googlesource.com/c/sys/+/24062/. This issue has been raised in go-tpm library google/go-tpm#146 and was fixed in google/go-tpm#147. Support for arm64 is only available starting v0.3.0 of the go-tpm library. So in order to build gke-exec-auth-plugin for arm64 we will need to update the version of the go-tpm library to v0.3.2.

All changes were produced by running the following command as recommended in the README.md

go get -u github.com/google/go-tpm && ./tools/update_vendor.sh

Tests were also updated because in go-tpm v0.3.0 validation was added to check that the magic value in the attestation data is always 0xff544347. We were not setting the value in our tests and so they were failing with the following error:

E0923 06:16:16.904904      12 node_csr_approver.go:506] deny CSR "TestValidators/validateTPMAttestation_with_API/good_0": parsing attestation data in CSR: incorrect magic value: 0

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 21, 2021
@vinayakankugoyal
Copy link
Contributor Author

/assign @mikedanese

@vinayakankugoyal
Copy link
Contributor Author

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Sep 21, 2021
@vinayakankugoyal
Copy link
Contributor Author

/retest

@cici37
Copy link
Contributor

cici37 commented Sep 23, 2021

cloud-provider-gcp-tests cloud-provider-gcp-e2e-full is blocked by #273

@vinayakankugoyal
Copy link
Contributor Author

cloud-provider-gcp-tests cloud-provider-gcp-e2e-full is blocked by #273

I think these failures have nothing to do with #273. cloud-provider-gcp-tests were failing because of new validation that was added to go-tmp. I have updated the description with the details of why the tests were failing.

@mikedanese
Copy link
Member

/lgtm
/approved

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 23, 2021
@mikedanese
Copy link
Member

/retest

@cici37
Copy link
Contributor

cici37 commented Sep 23, 2021

cloud-provider-gcp-tests cloud-provider-gcp-e2e-full is blocked by #273

I think these failures have nothing to do with #273. cloud-provider-gcp-tests were failing because of new validation that was added to go-tmp. I have updated the description with the details of why the tests were failing.

I was trying to say that if you push another commit and trigger cloud-provider-gcp-e2e-full again, it will fail because of #273 like what happened now -_-. I will try to get it merge asap.

@cici37
Copy link
Contributor

cici37 commented Sep 23, 2021

#273 is merged. Please fetch the latest changes which will unblock e2e failures. Thanks

@vinayakankugoyal
Copy link
Contributor Author

/retest

@mikedanese
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mikedanese, vinayakankugoyal

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

The pull request process is described 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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 23, 2021
@k8s-ci-robot k8s-ci-robot merged commit d054beb into kubernetes:master Sep 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants