build docs, helper scripts #192
Conversation
sipasing
force-pushed
the
project-docs
branch
2 times, most recently
from
6a1ca4a to
27806f3
Compare
|
@dbenoit17 Can you give me some feedback on what else can be added in this PR to improve this. I kept it minimum so we can add on it in subsequent PRs. |
docs/build.md
Outdated
|
|
||
| ## Env variables | ||
|
|
||
| `GOEXPERIMENT=strictfipsruntime` helps ensure a fips compliant binary is build. Note that this is functionally equivalent of |
docs/build.md
Outdated
| GO_BUILDTAGS+="goexperiment.strictfipsruntime" | ||
| ``` | ||
|
|
||
| You can export more Go flags like `GO_GCFLAGS="-N -l"` to add symbols in the final binary. |
There was a problem hiding this comment.
I would remove this, as it's documented in ./make.bash and isn't unique to this fork.
There was a problem hiding this comment.
This also disabled some classes of optimizations so it isn't recommended for normal builds.
docs/build.md
Outdated
|
|
||
| ## Native build - Method 1 | ||
|
|
||
| The easiest way to do is to simply clone this repo and run make. see a more comprehensive script at `scripts/build1.sh` |
There was a problem hiding this comment.
Capitalize 's' in 'see'.
There was a problem hiding this comment.
I would also say 'apply patches and from the internal go/src directory run ./make.bash' instead of just 'run make'.
docs/build.md
Outdated
|
|
||
| ## Native build - Method 2 | ||
|
|
||
| Another way is to directly apply fips patches on downloaded go and golang-fips binaries. see full script at `scripts/build2.sh` |
There was a problem hiding this comment.
Formatting: there's an extra space after 'patches on' and then capitalize any word after punctuation.
scripts/build1.sh
Outdated
| TOP_DIR=/tmp/golang-fips/build1 | ||
| [[ -d $TOP_DIR ]] || mkdir -p $TOP_DIR | ||
| cd $TOP_DIR | ||
| [[ -d golang-fips ]] || git clone https://github.com/golang-fips/go.git golang-fips |
There was a problem hiding this comment.
This seems wrong, why would we download another copy? If you're running this script you already have this repo.
There was a problem hiding this comment.
True, I thought of users copy pasting it before cloning this repo. But that wont make much of a use case. Let em update this.
| @@ -0,0 +1,43 @@ | |||
| FROM <distro-specific-base-image> | |||
There was a problem hiding this comment.
I'm not sure what benefit we get from having this Dockerfile in here. In fact, if it's just a sample and not meant to be runnable directly and used via any kind of CI or anything I can see it getting stagnant over time.
There was a problem hiding this comment.
Do u think it would help if we add distro specific files under a new scripts/distro folder and have that add over time.
distro/rh/Dockerfile
distro/centos/Dockerfile
There was a problem hiding this comment.
I think container images are better suited for downstream maintainers to handle. We don't intend to provide an upstream golang-fips container image, so we don't need to include a Dockerfile.
| @@ -0,0 +1 @@ | |||
| # Dependencies | |||
There was a problem hiding this comment.
Fill in with actual documentation, I think we just include it directly in this PR.
| @@ -0,0 +1 @@ | |||
| # OpenSSL considerations | |||
There was a problem hiding this comment.
Fill in with actual documentation, I think we just include it directly in this PR.
| @@ -0,0 +1 @@ | |||
| # Overview here | |||
There was a problem hiding this comment.
Fill in with actual documentation, I think we just include it directly in this PR.
| @@ -0,0 +1 @@ | |||
| # Test configuration | |||
There was a problem hiding this comment.
Fill in with actual documentation, I think we just include it directly in this PR.
| ``` | ||
| git clone https://github.com/golang-fips/go.git golang-fips | ||
| cd golang-fips | ||
| ./scripts/full-initialize-repo.sh go$GOLANG_VER |
There was a problem hiding this comment.
The default version described in go/config/versions.json is the only explicitly compatible upstream version at a given time. It would be preferable to recommend calling this script without the version-override argument and just use the default configuration.
| @@ -0,0 +1,43 @@ | |||
| FROM <distro-specific-base-image> | |||
There was a problem hiding this comment.
I think container images are better suited for downstream maintainers to handle. We don't intend to provide an upstream golang-fips container image, so we don't need to include a Dockerfile.
| @@ -0,0 +1,49 @@ | |||
| #! /usr/bin/env bash | |||
There was a problem hiding this comment.
I see why someone might find these scripts useful, but they appear rather configuration and workflow specific. Whereas the existing builds scripts are workflow agnostic and already capable of producing a Go toolchain with the same configuration. I would suggest to instead that we document a more generic disconnected build approach using the patches, and avoid adding additional scripts.
| @@ -0,0 +1,11 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
Improved documentation would be preferred over adding another script, as this is already attainable with the current build scripts.
This partially addresses #145
This PR aims to add instructions to build a go binary using patches supplied in this repo. More comprehensive bash scripts have also been added giving users ability to run them directly in their env.