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.

Sign up for GitHub

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

Jump to bottom

Update to Golang 1.9.4 #6326

Merged
merged 2 commits into from Feb 13, 2018
Merged

Update to Golang 1.9.4 #6326

merged 2 commits into from Feb 13, 2018

Conversation

ph
Copy link
Contributor

@ph ph commented Feb 8, 2018

Keep up with the latest version of Golang.

@ph
Copy link
Contributor Author

ph commented Feb 8, 2018

waiting for CI before tagging for review.

@ph ph added the libbeat label Feb 8, 2018
@ph ph added the in progress Pull request is currently in progress. label Feb 8, 2018
@ph
Copy link
Contributor Author

ph commented Feb 8, 2018

Since 1.9.4, go restrict the usage of cgo flags, I wanted to see what is currently breaking.

@andrewkroh
Copy link
Member

Check .appveyor.yml too.

@ph
Copy link
Contributor Author

ph commented Feb 8, 2018 •
edited

@andrewkroh It uses .go-version nevermind I think you are talking about the flags :D

@ph
Copy link
Contributor Author

ph commented Feb 8, 2018

I am thinking about upgrading the psutil lib, they have removed the custom ldflags, I've also notified the issues concerning the compiled with a newer version

@ph
Copy link
Contributor Author

ph commented Feb 8, 2018 •
edited

Some core package aren't updated for this issue, see

./vendor/golang.org/x/sys/unix/zsyscall_solaris_amd64.go:13:3: //go:cgo_import_dynamic libc_pipe pipe "libc.so" only allowed in cgo-generated code
../vendor/golang.org/x/sys/unix/zsyscall_solaris_amd64.go:14:3: //go:cgo_import_dynamic libc_getsockname getsockname "libsocket.so" only allowed in cgo-generated code
../vendor/golang.org/x/sys/unix/zsyscall_solaris_amd64.go:15:3: //go:cgo_import_dynamic libc_getcwd getcwd "libc.so" only allowed in cgo-generated code
../vendor/golang.org/x/sys/unix/zsyscall_solaris_amd64.go:16:3: //go:cgo_import_dynamic libc_getgroups getgroups "libc.so" only allowed in cgo-generated code
../vendor/golang.org/x/sys/unix/zsyscall_solaris_amd64.go:17:3: //go:cgo_import_dynamic libc_setgroups setgroups "libc.so" only allowed in cgo-generated code
../vendor/golang.org/x/sys/unix/zsyscall_solaris_amd64.go:18:3: //go:cgo_import_dynamic libc_wait4 wait4 "libc.so" only allowed in cgo-generated code
../vendor/golang.org/x/sys/unix/zsyscall_solaris_amd64.go:19:3: //go:cgo_import_dynamic libc_gethostname gethostname "libc.so" only allowed in cgo-generated code
../vendor/golang.org/x/sys/unix/zsyscall_solaris_amd64.go:20:3: //go:cgo_import_dynamic libc_utimes utimes "libc.so" only allowed in cgo-generated code
../vendor/golang.org/x/sys/unix/zsyscall_solaris_amd64.go:21:3: //go:cgo_import_dynamic libc_utimensat utimensat "libc.so" only allowed in cgo-generated code
../vendor/golang.org/x/sys/unix/zsyscall_solaris_amd64.go:22:3: //go:cgo_import_dynamic libc_fcntl fcntl "libc.so" only allowed in cgo-generated code

@ph
Copy link
Contributor Author

ph commented Feb 8, 2018

Weird psutil should have worked, the flag is in the whitelist.

@tylerjl tylerjl mentioned this pull request Feb 8, 2018
Closed
@ruflin
Copy link
Member

ruflin commented Feb 8, 2018

@ph Seeing that you updated some x packages, this PR is kind of related by @cyrilleverrier #6246

@ph
Copy link
Contributor Author

ph commented Feb 8, 2018 via email •
edited

@ph ph force-pushed the fix/update-golang1.4 branch 3 times, most recently from 30de83e to e889e67 Compare February 9, 2018 17:14
@ph ph added review needs_backport PR is waiting to be backported to other branches. and removed in progress Pull request is currently in progress. labels Feb 9, 2018
kvch
kvch reviewed Feb 9, 2018
View reviewed changes
libbeat/scripts/Makefile
@@ -52,7 +52,7 @@ NOSETESTS_OPTIONS?=--process-timeout=$(TIMEOUT) --with-timer -v --with-xunit --x
TEST_ENVIRONMENT?=false ## @testing if true, "make testsuite" runs integration tests and system tests in a dockerized test environment
SYSTEM_TESTS?=false ## @testing if true, "make test" and "make testsuite" run unit tests and system tests
STRESS_TESTS?=false ## @testing if true, "make test" and "make testsuite" run also run the stress tests
GOX_OS?=linux darwin windows solaris freebsd netbsd openbsd ## @Building List of all OS to be supported by "make crosscompile".
Copy link
Contributor

Choose a reason for hiding this comment

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

An "f" is missing from "freebsd".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤕

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

@ruflin
Copy link
Member

ruflin commented Feb 11, 2018

@ph Should we open an issue to readd solaris as soon as the above issue is fixed?

@ruflin
Copy link
Member

ruflin commented Feb 11, 2018

@ph One more question: You have added the backport label. Not sure what the advantage would be to backport this to 6.2? Brew?

ruflin
ruflin reviewed Feb 11, 2018
View reviewed changes
vendor/vendor.json Outdated
@@ -1025,6 +1025,14 @@
"revision": "5bf94b69c6b68ee1b541973bb8e1144db23a194b",
"revisionTime": "2017-03-21T23:07:31Z"
},
{
Copy link
Member

Choose a reason for hiding this comment

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

I wonder why this is added as I does not seem we use the "top level" gopsutil package anywhere. How did you "update" gopsutil?

@ph
Update to Golang 1.9.4
5fc93c6 
Keep up with the latest version of Golang. Version prior to 1.9.4 are
affected by security issues on build. `go get` could have been used to do unwanted
code execution[1] CVE-2018-6574 during build.

The changes in golang now enforce more restrictive pragma declaration
when linking and compiling.

This commits, disable crosscompiling under solaris which requires an
update on the standard library.

```
../vendor/golang.org/x/sys/unix/zsyscall_solaris_amd64.go:13:3: //go:cgo_import_dynamic libc_pipe pipe "libc.so" only allowed in cgo-generated code
../vendor/golang.org/x/sys/unix/zsyscall_solaris_amd64.go:14:3: //go:cgo_import_dynamic libc_getsockname getsockname "libsocket.so" only allowed in cgo-generated code
```

We also update popsutil, thag was incorrectly defining an ldflags in a
previous release making the build fails under the new restriction.

[1] golang/go#23672
@ph
update vendor correctly
00cadd7
@ph
Copy link
Contributor Author

ph commented Feb 12, 2018

@ruflin good catch, I've learned today.

@ph One more question: You have added the backport label. Not sure what the advantage would be to backport this to 6.2? Brew?

For two reason:

@ph ph mentioned this pull request Feb 12, 2018
Closed
@ph
Copy link
Contributor Author

ph commented Feb 12, 2018

@ruflin I've created the issue #6359 for Solaris.

@ph
Copy link
Contributor Author

ph commented Feb 13, 2018

@ruflin Is there anything missing in this PR?

@ruflin ruflin merged commit 9e46667 into elastic:master Feb 13, 2018
@ph
Copy link
Contributor Author

ph commented Feb 13, 2018 via email

@ph ph removed the needs_backport PR is waiting to be backported to other branches. label Feb 13, 2018
@ph
Copy link
Contributor Author

ph commented Feb 13, 2018

Currently we have decided not to backport it to 6.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruflin ruflin ruflin left review comments

@kvch kvch kvch left review comments

Assignees
No one assigned
Labels
libbeat review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ph @andrewkroh @ruflin @kvch