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

Support building on Windows #191

Merged
merged 8 commits into from
Apr 14, 2022
Merged

Support building on Windows #191

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e356db7
Add support to build on Windows
amartinezfayo Apr 11, 2022
e05637f
Upodate GitHub Workflow
amartinezfayo Apr 11, 2022
3bd0311
Add support to build on Windows
amartinezfayo Apr 11, 2022
dc6547f
Add support to build on Windows
amartinezfayo Apr 11, 2022
e9d9dc6
Merge remote-tracking branch 'origin/main' into windows-build
amartinezfayo Apr 13, 2022
28015be
Add support to build on Windows
amartinezfayo Apr 13, 2022
4f6dde8
Add support to build on Windows
amartinezfayo Apr 13, 2022
6463959
Update success job
amartinezfayo Apr 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Set autocrlf = false
* -text
71 changes: 65 additions & 6 deletions .github/workflows/pr_build.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ on:
env:
GO_VERSION: 1.13
jobs:
lint:
lint-linux:
runs-on: ubuntu-latest
steps:
- name: Checkout
Expand All @@ -16,7 +16,8 @@ jobs:
go-version: ${{ env.GO_VERSION }}
- name: Lint
run: make lint
test:

test-linux:
runs-on: ubuntu-latest
steps:
- name: Checkout
Expand All @@ -27,12 +28,70 @@ jobs:
go-version: ${{ env.GO_VERSION }}
- name: Test
run: make test

lint-windows:
runs-on: windows-2022
defaults:
run:
shell: msys2 {0}
steps:
- name: Checkout
uses: actions/checkout@v2
- name: Setup go
uses: actions/setup-go@v2
with:
go-version: ${{ env.GO_VERSION }}
- name: Install msys2
uses: msys2/setup-msys2@v2
with:
msystem: MINGW64
update: true
install: >-
git
base-devel
mingw-w64-x86_64-toolchain
unzip
- name: Lint
run: make lint

test-windows:
runs-on: windows-2022
defaults:
run:
shell: msys2 {0}
steps:
- name: Checkout
uses: actions/checkout@v2
- name: Setup go
uses: actions/setup-go@v2
with:
go-version: ${{ env.GO_VERSION }}
- name: Install msys2
uses: msys2/setup-msys2@v2
with:
msystem: MINGW64
update: true
install: >-
git
base-devel
mingw-w64-x86_64-toolchain
unzip
- name: Test
run: make test

# This job is just here to make sure that the other jobs have completed
# and is used as a single job to block PR merge from. GH doesn't have a
# way to say "all jobs from this action", which would be ideal.
success:
needs: [lint, test]
success-linux:
Copy link
Collaborator

Choose a reason for hiding this comment

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

may we put all need in the same job? if we separate between windows/linux, we'll not have a single job that group all.

success:
    needs: [lint-linux, test-linux, lint-windows, test-windows]
    runs-on: ubuntu-latest
    steps:
      - name: Shout it out
        run: echo SUCCESS
      - name: Shout it out - Linux
        run: echo SUCCESS - Linux

needs: [lint-linux, test-linux]
runs-on: ubuntu-latest
steps:
- name: Shout it out
run: echo SUCCESS
- name: Shout it out - Linux
run: echo SUCCESS - Linux

success-windows:
needs: [lint-windows, test-windows]
runs-on: windows-2022
steps:
- name: Shout it out - Windows
run: echo SUCCESS - Windows
35 changes: 28 additions & 7 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ os2=osx
else ifeq ($(os1),Linux)
os1=linux
os2=linux
else ifeq (,$(findstring MYSYS_NT-10-0-, $(os1)))
os1=windows
os2=windows
else
$(error unsupported OS: $(os1))
endif
Expand All @@ -47,7 +50,9 @@ endif
build_dir := ${CURDIR}/.build/$(os1)-$(arch1)

protoc_version = 3.14.0
ifeq ($(arch1),aarch64)
ifeq ($(os1),windows)
protoc_url = https://github.com/protocolbuffers/protobuf/releases/download/v$(protoc_version)/protoc-$(protoc_version)-win64.zip
else ifeq ($(arch1),aarch64)
protoc_url = https://github.com/protocolbuffers/protobuf/releases/download/v$(protoc_version)/protoc-$(protoc_version)-$(os2)-aarch_64.zip
else
protoc_url = https://github.com/protocolbuffers/protobuf/releases/download/v$(protoc_version)/protoc-$(protoc_version)-$(os2)-$(arch1).zip
Expand Down Expand Up @@ -79,12 +84,28 @@ apiprotos := \
go_version_full := 1.13.15
go_version := $(go_version_full:.0=)
go_dir := $(build_dir)/go/$(go_version)
go_bin_dir := $(go_dir)/bin
go_url = https://storage.googleapis.com/golang/go$(go_version).$(os1)-$(arch2).tar.gz

ifeq ($(os1),windows)
go_bin_dir = $(go_dir)/go/bin
go_url = https://storage.googleapis.com/golang/go$(go_version).$(os1)-$(arch2).zip
exe=".exe"
else
go_bin_dir = $(go_dir)/bin
go_url = https://storage.googleapis.com/golang/go$(go_version).$(os1)-$(arch2).tar.gz
exe=
endif

go_path := PATH="$(go_bin_dir):$(PATH)"

go-check:
ifneq (go$(go_version), $(shell $(go_path) go version 2>/dev/null | cut -f3 -d' '))
ifeq (go$(go_version), $(shell $(go_path) go version 2>/dev/null | cut -f3 -d' '))
else ifeq ($(os1),windows)
@echo "Installing go$(go_version)..."
rm -rf $(dir $(go_dir))
mkdir -p $(go_dir)
curl -o $(go_dir)\go.zip -sSfL $(go_url)
unzip -qq $(go_dir)\go.zip -d $(go_dir)
else
@echo "Installing go$(go_version)..."
$(E)rm -rf $(dir $(go_dir))
$(E)mkdir -p $(go_dir)
Expand All @@ -97,8 +118,8 @@ endif
#############################################################################

.PHONY: lint
lint: $(golangci_lint_bin)
@cd ./v2; $(golangci_lint_bin) run ./...
lint: $(golangci_lint_bin) | go-check
@cd ./v2; PATH="$(go_bin_dir):$(PATH)" $(golangci_lint_bin) run ./...

$(golangci_lint_bin):
@echo "Installing golangci-lint $(golangci_lint_version)..."
Expand All @@ -111,7 +132,7 @@ $(golangci_lint_bin):
#############################################################################

.PHONY: test
test:
test: | go-check
@cd ./v2; $(go_path) go test ./...

#############################################################################
Expand Down
11 changes: 6 additions & 5 deletions v2/bundle/jwtbundle/bundle_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/spiffe/go-spiffe/v2/bundle/jwtbundle"
"github.com/spiffe/go-spiffe/v2/internal/test"
"github.com/spiffe/go-spiffe/v2/internal/test/errstrings"
"github.com/spiffe/go-spiffe/v2/spiffeid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand All @@ -21,18 +22,18 @@ type testFile struct {
var (
td = spiffeid.RequireTrustDomainFromString("example.org")
testFiles = map[string]testFile{
"valid 1": testFile{
"valid 1": {
filePath: "testdata/jwks_valid_1.json",
keysCount: 1,
},
"valid 2": testFile{
"valid 2": {
filePath: "testdata/jwks_valid_2.json",
keysCount: 2,
},
"non existent file": testFile{
"non existent file": {
filePath: "testdata/does-not-exist.json",
},
"missing kid": testFile{
"missing kid": {
filePath: "testdata/jwks_missing_kid.json",
},
}
Expand Down Expand Up @@ -68,7 +69,7 @@ func TestLoad(t *testing.T) {
},
{
tf: testFiles["non existent file"],
err: "jwtbundle: unable to read JWT bundle: open testdata/does-not-exist.json: no such file or directory",
err: "jwtbundle: unable to read JWT bundle: open testdata/does-not-exist.json: " + errstrings.FileNotFound,
},
{
tf: testFiles["missing kid"],
Expand Down
3 changes: 2 additions & 1 deletion v2/bundle/spiffebundle/bundle_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/spiffe/go-spiffe/v2/bundle/spiffebundle"
"github.com/spiffe/go-spiffe/v2/bundle/x509bundle"
"github.com/spiffe/go-spiffe/v2/internal/test"
"github.com/spiffe/go-spiffe/v2/internal/test/errstrings"
"github.com/spiffe/go-spiffe/v2/spiffeid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -70,7 +71,7 @@ func TestNew(t *testing.T) {
}

func TestLoad(t *testing.T) {
testCases[0].err = "spiffebundle: unable to read SPIFFE bundle: open testdata/does-not-exist.json: no such file or directory"
testCases[0].err = "spiffebundle: unable to read SPIFFE bundle: open testdata/does-not-exist.json: " + errstrings.FileNotFound

for _, testCase := range testCases {
testCase := testCase
Expand Down
9 changes: 9 additions & 0 deletions v2/internal/test/errstrings/err_posix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//go:build !windows
// +build !windows

// OS specific error strings
package errstrings
Copy link
Member

Choose a reason for hiding this comment

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

This isn't my favorite thing, but I think is fine in the short term. I think we should probably change the tests to uses errors.Is or something to detect the right error condition, but I recognize that is a larger change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I agree. I explored that path and found it out of scope for this PR, but would really like to have it implemented.


var (
FileNotFound = "no such file or directory"
)
9 changes: 9 additions & 0 deletions v2/internal/test/errstrings/err_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//go:build windows
// +build windows

// OS specific error strings
package errstrings

var (
FileNotFound = "The system cannot find the file specified."
)