Kuadrant / authorino

K8s-native AuthN/AuthZ service to protect your APIs.
Apache License 2.0
201 stars 32 forks source link

add commit hash implementation similar to kuadrantctl to authorino #473

Closed ehearneRedHat closed 2 months ago

ehearneRedHat commented 4 months ago

What:

Verify:

NOTE: for running the binaries use authorino version to check implementation. Within container in default namespace, check the logs at the top for the version.

  1. Run make build and make local-setup locally. Without any file changes made the hash should be displayed without appendixed dirty flag. Then add a comment to a file to have git detect new changes and re-run the commands checking binary and in-cluster for both respectively.
  2. Run the build-images.yaml workflow and check that the binaries show the hash.
ehearneRedHat commented 3 months ago

@guicassolato re-requesting your review... I didn't add the version to the workflow file as requested .

guicassolato commented 3 months ago

@ehearneRedHat, I think I'm missing something...

I still see the Authorino version with -dev suffix hard-coded in the Makefile for the docker-build arg. I thought we'd be reading it from the build.yaml. 🤔

The build-images workflow on the other hand seems to be setting VERSION always to latest, even when we build for feature branches and tag releases!

I guess what I expected was:

main feature branch vX.Y.Z
Version latest <branch-name> X.Y.Z
Image tags latest, <git-sha> <branch-name>, <git-sha> vX.Y.Z, <git-sha>

It should not matter if we build the image locally (dev env) or in a GHA. The outcome should be the same, depending only on what we are building, i.e., the code currently in main, in a feature branch, or "vX.Y.Z"-tagged (released version).

The build.yaml file should be just a resource to achieve that, if needed and to avoid hard-coding version numbers inside a Makefile or a Golang file.

Another scenario – which I do not like, but could live with if the rest of Kuadrant so decides:

main feature branch vX.Y.Z
Version <curr+1>-dev <curr+1>-dev X.Y.Z
Image tags latest, <git-sha> <branch-name>, <git-sha> vX.Y.Z, <git-sha>
ehearneRedHat commented 3 months ago

@ehearneRedHat, I think I'm missing something...

I still see the Authorino version with -dev suffix hard-coded in the Makefile for the docker-build arg. I thought we'd be reading it from the build.yaml. 🤔

The build-images workflow on the other hand seems to be setting VERSION always to latest, even when we build for feature branches and tag releases!

I guess what I expected was:

main feature branch vX.Y.Z Version latest X.Y.Z Image tags latest, , vX.Y.Z, It should not matter if we build the image locally (dev env) or in a GHA. The outcome should be the same, depending only on what we are building, i.e., the code currently in main, in a feature branch, or "vX.Y.Z"-tagged (released version).

The build.yaml file should be just a resource to achieve that, if needed and to avoid hard-coding version numbers inside a Makefile or a Golang file.

Another scenario – which I do not like, but could live with if the rest of Kuadrant so decides:

main feature branch vX.Y.Z Version <curr+1>-dev <curr+1>-dev X.Y.Z Image tags latest, , vX.Y.Z,

@guicassolato you're absolutely right! I did forget to amend the docker-build target.

My understanding was that you would prefer not touching the version within the build-images.yaml workflow . That's why I didn't adjust.

I can see now what you were expecting, and how the current state isn't aligned with that! I think it sounds like a good idea. There would need to be some logic to check for the branches but I'll implement it!

ehearneRedHat commented 3 months ago

@guicassolato I believe I made the requested changes. Feel free to review when you have the chance :)

ehearneRedHat commented 3 months ago

@guicassolato feel free to re-review again, but I need to check the verification steps first. You are too fast ! :D

ehearneRedHat commented 3 months ago

https://github.com/Kuadrant/authorino/actions/runs/10507966761

ehearneRedHat commented 2 months ago

@guicassolato I have amended changes - feel free to review. :)

ehearneRedHat commented 2 months ago

I pass the baton over to @guicassolato to complete this task, as today marks my last day of the internship in Kuadrant at Red Hat.

Thank you to everyone who has helped me over the eight months of my being here. It was an amazing experience and hope to be back someday soon. :v:

guicassolato commented 2 months ago

I pass the baton over to @guicassolato to complete this task, as today marks my last day of the internship in Kuadrant at Red Hat.

Thank you to everyone who has helped me over the eight months of my being here. It was an amazing experience and hope to be back someday soon. ✌️

Thank you so much for this, @ehearneRedHat! I will own the PR now, though the credit is still all yours, if you ask me!