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

chore: Adding version info for gk, opa, and frameworks in gator cmd #2338

Merged
merged 1 commit into from
Oct 17, 2022

Conversation

JaydipGabani
Copy link
Contributor

@JaydipGabani JaydipGabani commented Oct 14, 2022

Signed-off-by: Jaydip Gabani gabanijaydip@gmail.com

What this PR does / why we need it:
Adds more information in output of gator --version

Which issue(s) this PR fixes (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Fixes #1643

Special notes for your reviewer:

Result:

gator --version
gator version 
Gator & Gatekeeper version: v3.10.0-beta.2, OPA version: 0.44.0, Framework Version v0.0.0-20221006234738-a3d297b3152f

@sozercan
Copy link
Member

@JaydipGabani looks like tests are failing

@JaydipGabani
Copy link
Contributor Author

@sozercan we are good to go for reviews!

go.mod Outdated
@@ -27,6 +27,7 @@ require (
github.com/onsi/gomega v1.19.0
github.com/open-policy-agent/cert-controller v0.4.0
github.com/open-policy-agent/frameworks/constraint v0.0.0-20221006234738-a3d297b3152f
github.com/open-policy-agent/opa v0.44.0
Copy link
Member

Choose a reason for hiding this comment

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

we don't want to add opa version here but get the dependency through frameworks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So fetch the version from mod file similar to frameworks/constraint ?

gator.Dockerfile Outdated
@@ -22,7 +22,7 @@ COPY . /tmp/gatekeeper

WORKDIR /tmp/gatekeeper/cmd/gator

RUN go build -mod vendor -a -ldflags "${LDFLAGS:--X github.com/open-policy-agent/gatekeeper/pkg/version.Version=latest}" -o /gator
RUN go build -mod vendor -a -ldflags "${LDFLAGS}" -o /gator
Copy link
Member

Choose a reason for hiding this comment

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

let's keep the default if LDFLAGS is not passed

Makefile Outdated
@@ -22,6 +22,7 @@ BATS_TESTS_FILE ?= test/bats/test.bats
HELM_VERSION ?= 3.7.2
NODE_VERSION ?= 16-bullseye-slim
YQ_VERSION ?= 4.2.0
FRAMEWORK_VERSION ?= $(shell go list -f '{{ .Version }}' -m github.com/open-policy-agent/frameworks/constraint)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
FRAMEWORK_VERSION ?= $(shell go list -f '{{ .Version }}' -m github.com/open-policy-agent/frameworks/constraint)
FRAMEWORKS_VERSION ?= $(shell go list -f '{{ .Version }}' -m github.com/open-policy-agent/frameworks/constraint)

Makefile Outdated
@@ -45,6 +46,8 @@ LDFLAGS := "-X github.com/open-policy-agent/gatekeeper/pkg/version.Version=$(VER
-X github.com/open-policy-agent/gatekeeper/pkg/version.Timestamp=$(BUILD_TIMESTAMP) \
-X github.com/open-policy-agent/gatekeeper/pkg/version.Hostname=$(BUILD_HOSTNAME)"

GTRFLAGS := ${LDFLAGS} "-X main.frameworkVersion=$(FRAMEWORK_VERSION)"
Copy link
Member

Choose a reason for hiding this comment

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

i think we can add frameworksVersion to LDFLAGS as it applies there too

"github.com/spf13/cobra"
)

const version = "alpha"
var frameworkVersion = "v0.0.0-20221006234738-a3d297b3152f"
Copy link
Member

Choose a reason for hiding this comment

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

how does this get set? can we skip hardcoding this?

@codecov-commenter
Copy link

Codecov Report

Base: 53.45% // Head: 53.50% // Increases project coverage by +0.04% 🎉

Coverage data is based on head (3923ef0) compared to base (07c6d25).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2338      +/-   ##
==========================================
+ Coverage   53.45%   53.50%   +0.04%     
==========================================
  Files         116      116              
  Lines       10174    10174              
==========================================
+ Hits         5439     5444       +5     
+ Misses       4317     4313       -4     
+ Partials      418      417       -1     
Flag Coverage Δ
unittests 53.50% <ø> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...onstrainttemplate/constrainttemplate_controller.go 57.65% <0.00%> (+1.19%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Makefile Outdated
-t ${GATOR_REPOSITORY}:${DEV_TAG} \
-t ${GATOR_REPOSITORY}:dev \
-f gator.Dockerfile . --push
-f gator.Dockerfile . --load
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to revert this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!! I missed this

Makefile Outdated
@@ -367,10 +371,10 @@ docker-buildx-crds-release: build-crds docker-buildx-builder

# Build gator image
docker-buildx-gator-dev: docker-buildx-builder
docker buildx build --build-arg LDFLAGS=${LDFLAGS} --platform "linux/amd64,linux/arm64,linux/arm/v6"\
Copy link
Contributor

Choose a reason for hiding this comment

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

Were these platform versions removed for local testing? If so should we add this back?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!! I missed this

@@ -24,7 +28,7 @@ func init() {
var rootCmd = &cobra.Command{
Use: "gator subcommand",
Short: "gator is a suite of authorship tools for Gatekeeper",
Version: version,
Version: fmt.Sprintf("\nGator & Gatekeeper version: %s, OPA version: %s, Framework version: %s", version.Version, opaVersion, frameworksVersion),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Version: fmt.Sprintf("\nGator & Gatekeeper version: %s, OPA version: %s, Framework version: %s", version.Version, opaVersion, frameworksVersion),
Version: fmt.Sprintf("\nGator version: %s (Feature State: %s), OPA version: %s, Framework version: %s", version.Version, state, opaVersion, frameworksVersion),

state is alpha

Comment on lines 14 to 18
var frameworksVersion string

var opaVersion string
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
var frameworksVersion string
var opaVersion string
var (
frameworksVersion string
opaVersion string
)

Signed-off-by: Jaydip Gabani <gabanijaydip@gmail.com>
Copy link
Member

@sozercan sozercan left a comment

Choose a reason for hiding this comment

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

Thank you! LGTM

Copy link
Contributor

@maxsmythe maxsmythe left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for the PR!

@maxsmythe maxsmythe merged commit 3ce399a into open-policy-agent:master Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gator cli version output
5 participants