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

Refactor the gittag plugin + fix sources of gittag #457

Merged
merged 17 commits into from
Jan 17, 2022

Conversation

dduportal
Copy link
Contributor

@dduportal dduportal commented Jan 12, 2022

This PR comes from the work in progress in #412 and refactors the gittag plugin into its own package.

While working on this, 2 bugs where found on this plugin and reported (#467 #468). These 2 bugs are now corrected

Fix #467
Fix #469

Test

To test this pull request, you can run the following commands:

go build -o ./dist/updatecli && ./dist/updatecli diff --config examples/updatecli.generic/gitTag.yaml

should error only on the 2 targets (expected) with:

ERROR: Git push tag error: authentication required
ERROR: Something went wrong in target "git" :
ERROR: authentication required

Additional Information

  • Please note that I did work on the conditions/targets, neither on unit tests for this one
    • factorizing code on condition and target between "normal" and "SCM" methods
    • Using unit test with mock
  • The fix embeded on this PR would allow the scenario "get latest stable nginx" (it wasn't possible before):
title: Bump nginx:stable docker image version

scms:
  default:
    kind: github
    spec:
      user: "{{ .github.user }}"
      email: "{{ .github.email }}"
      owner: "{{ .github.owner }}"
      repository: "{{ .github.repository }}"
      token: "{{ requiredEnv .github.token }}"
      username: "{{ .github.username }}"
      branch: "{{ .github.branch }}"
  nginx-gh-mirror:
    kind: git
    spec:
      url: "https://github.com/nginx/nginx.git"
      branch: "master"

sources:
  latestRelease:
    name: Get latest stable version of nginx
    kind: gitTag
    scmID: nginx-gh-mirror
    spec:
      versionFilter:
        kind: regex
        ## Nginx stable version have the minor digit as an even number
        pattern: 'release-(\d+)\.(\d*[0|2|4|6|8])\.(\d+)'
    transformers:
      - trimPrefix: "release-"
      - addSuffix: "-alpine"

@dduportal dduportal changed the title Chore: refactor gittag plugin Refactor + Fix the gittag plugin Jan 15, 2022
@dduportal dduportal added bug Something isn't working condition Modify condition resource resource-gittag Resource of kind Git Tag source target Related to updatecli target labels Jan 15, 2022
@dduportal dduportal changed the title Refactor + Fix the gittag plugin Refactor the gittag plugin + fix sources of gittag Jan 15, 2022
@dduportal dduportal marked this pull request as ready for review January 15, 2022 18:51
@dduportal
Copy link
Contributor Author

Special addon for you @olblak : after merging #419, I've added a e2e test for gittag sources :)

@dduportal dduportal force-pushed the chore/refactor-git branch 2 times, most recently from 18a47e5 to 56e073c Compare January 16, 2022 11:08
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Fixes updatecli#467.

The fix comes from stop holding state in version object to avoid reference/copy issues.
Instead, the method "filer.Search()" returns the object Version by copy (struct of 2 strings, it's fine for memory).
It also allows to returns early when needed.

Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Fixes updatecli#469.

This fix implies extracting the time of creation of each tag reference
and sort by time before answering the list of tags.

Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
…t of disablesourceinput

Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
lemeurherve
lemeurherve previously approved these changes Jan 17, 2022
Copy link
Member

@lemeurherve lemeurherve left a comment

Choose a reason for hiding this comment

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

Small nipticks, LGTM otherwise!

Co-authored-by: Hervé Le Meur <91831478+lemeurherve@users.noreply.github.com>
@dduportal dduportal dismissed stale reviews from lemeurherve and olblak via 22f0875 January 17, 2022 20:27
@dduportal dduportal enabled auto-merge (rebase) January 17, 2022 20:28
@dduportal dduportal enabled auto-merge (squash) January 17, 2022 20:28
@dduportal
Copy link
Contributor Author

Thanks for the review folks! I've enabled auto-merge (with squash).

I've taken in account almost all feedbacks (except 2: I've added comments).

As soon as you approve, this will be merged : subsequent comment will need ...a subsequent PR ;)

@dduportal dduportal merged commit 1b9df70 into updatecli:main Jan 17, 2022
@dduportal dduportal deleted the chore/refactor-git branch January 17, 2022 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working condition Modify condition resource resource-gittag Resource of kind Git Tag source target Related to updatecli target
Projects
None yet
3 participants