Page MenuHomeFreeBSD

Improve security/teleport build
ClosedPublic

Authored by seanc on May 6 2018, 12:00 AM.
Tags
None
Referenced Files
F81589656: D15311.id42175.diff
Thu, Apr 18, 2:33 PM
F81589652: D15311.id44742.diff
Thu, Apr 18, 2:33 PM
F81589651: D15311.id42331.diff
Thu, Apr 18, 2:33 PM
F81589648: D15311.id42593.diff
Thu, Apr 18, 2:33 PM
F81589646: D15311.id42431.diff
Thu, Apr 18, 2:33 PM
F81589643: D15311.id44737.diff
Thu, Apr 18, 2:33 PM
F81589641: D15311.id.diff
Thu, Apr 18, 2:33 PM
F81589639: D15311.id42598.diff
Thu, Apr 18, 2:33 PM
Subscribers
None

Details

Summary

Upgrade gravitational teleport to 2.6.6.

Explicitly specify the git sha when building teleport. Restrict builds to amd64.

Test Plan

doas poudriere testport -j 12amd64 -o security/teleport -p dev

Result is clean.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

You need to bump PORTREVISION.

Also, could you use devel/arcanist, or at least generate a diff with full context like it does, with svn diff -x -U9999 or git diff -U9999.

seanc edited the summary of this revision. (Show Details)

Upgrade gravitational teleport to 2.5.7.

Explicitly specify the git sha when building teleport. Restrict builds to amd64.

In D15311#323813, @mat wrote:

You need to bump PORTREVISION.

Also, could you use devel/arcanist, or at least generate a diff with full context like it does, with svn diff -x -U9999 or git diff -U9999.

2.5.7 was released so I bumped that the PORTVERSION instead.

This revision is now accepted and ready to land.May 11 2018, 7:59 PM
This revision was automatically updated to reflect the committed changes.
head/security/teleport/Makefile
13

Please add ONLY_FOR_ARCHS_REASON to explain.

15–17

Remove ${LOCALBASE}/bin/.

23

Remove, default.

32

Use @sample?

35

Why?

46

It would probably be easier to simply do something like:

's|^GITREF=.*|GITREF=${GH_TAG_COMMIT}|'

and remove the patch.

55

Remove -s, builds must not be silent.

seanc marked 7 inline comments as done.

Updated port with feedback from @mat

head/security/teleport/Makefile
15–17

I looked for a way to see if there is a test to verify that a version of go is greater than a given version. If a user only has go1.4 in their path then the build will fail, whereas using ${LOCALBASE}/bin implies someone is keeping current with their ports maintenance.

I've made the adjustment, but that's why I used ${LOCALBASE}/bin.

35

The teleport binary includes extra UI elements appended to the binary that is removed when the binary is stripped.

Could you use devel/arcanist, or at least generate a diff with full context like it does, with svn diff -x -U9999 or git diff -U9999.

head/security/teleport/Makefile
15–17

I don't understand. If you remove ${LOCALBASE}/bin/ as I asked, the framework will look for a binary named go in PATH. If a user has a go14 binary, well, it will not be used, as it is not called go.

Please remove ${LOCALBASE}/bin/.

35

Then please add a comment before the STRIP variable saying so.

seanc marked 4 inline comments as done.

Paste with context

head/security/teleport/Makefile
15–17

I understand. My comment is with regards to common development patterns where ~/go/bin/go may not be the latest and greatest and may not meet the build requirements. It's common to use PATH=$GOPATH/bin:$PATH which means a user's local go would be used. I was trying to guard against that particular outcome. I fixed it in the patch but was explaining why I used ${LOCALBASE}/bin/go vs just go.

Mmmm, right, but I really do not feel this edge case needs to be addressed, packages are supposed to be built in a clean environment in poudriere/synth, or as root, I think it's safe to assume that there will not be a ~root/.../go.

Agreed, and I have already removed ${LOCALBASE}/bin/go from BUILD_DEPENDS - I wasn't arguing with you, I was only explaining why I added it in the first place. :)

seanc edited the summary of this revision. (Show Details)

Incorporate @swills' feedback. Work around daemon(8)'s lack of syslog functionality in 10-STABLE.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 2 2018, 5:26 AM
This revision was automatically updated to reflect the committed changes.
head/security/teleport/Makefile
48–49

Please use -exec ... + instead of -exec ... ;, also, fold the two sed commands into one line. It will make it run one or two sed commands instead of twice the number of files there are in WRKDIR.

59–60

run sed only once.