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

install-script:feature - add install-beta and install-rc #827

Merged
merged 1 commit into from
Dec 1, 2021
Merged

install-script:feature - add install-beta and install-rc #827

merged 1 commit into from
Dec 1, 2021

Conversation

iancardosozup
Copy link
Contributor

Signed-off-by: Ian Cardoso ian.cardoso@zup.com.br

- What I did

- How to verify it

- Description for the changelog

matheusalcantarazup
matheusalcantarazup suggested changes Nov 29, 2021
View reviewed changes
Makefile Outdated Show resolved Hide resolved
@iancardosozup iancardosozup force-pushed the feature/add-make-install-beta-and-rc branch 2 times, most recently from f492cbf to 1d3e584 Compare November 29, 2021 14:30
wiliansilvazup
wiliansilvazup suggested changes Nov 29, 2021
View reviewed changes
Copy link
Contributor

@wiliansilvazup wiliansilvazup left a comment

Choose a reason for hiding this comment

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

Create handle in case the command responds with failed or empty status, and change envs LATEST_RC and LATEST_BETA into the shell script so that the user passes the parameter latest or beta-latest or rc-latest

@iancardosozup iancardosozup force-pushed the feature/add-make-install-beta-and-rc branch 2 times, most recently from ca52c9d to 624c7a8 Compare November 29, 2021 17:21
@iancardosozup
Copy link
Contributor Author

Create handle in case the command responds with failed or empty status, and change envs LATEST_RC and LATEST_BETA into the shell script so that the user passes the parameter latest or beta-latest or rc-latest

if someone wants to pass latest or a specific version as a parameter they can use the existing script, no? i created install-beta-rc.sh to receive only rc or beta arguments. Should i make a change to use a specific beta/rc version?

@matheusalcantarazup matheusalcantarazup dismissed their stale review November 29, 2021 17:36

New files was added since approval

@matheusalcantarazup
Copy link
Contributor

if someone wants to pass latest or a specific version as a parameter they can use the existing script, no? i created install-beta-rc.sh to receive only rc or beta arguments. Should i make a change to use a specific beta/rc version?

I think that would be better change the existing script. This new script looks very similar with the current.

@iancardosozup
Copy link
Contributor Author

if someone wants to pass latest or a specific version as a parameter they can use the existing script, no? i created install-beta-rc.sh to receive only rc or beta arguments. Should i make a change to use a specific beta/rc version?

I think that would be better change the existing script. This new script looks very similar with the current.

someone could be using the existing script passing an specific version, it would break them if we change the arguments to latest, beta or rc

@matheusalcantarazup
Copy link
Contributor

if someone wants to pass latest or a specific version as a parameter they can use the existing script, no? i created install-beta-rc.sh to receive only rc or beta arguments. Should i make a change to use a specific beta/rc version?

I think that would be better change the existing script. This new script looks very similar with the current.

someone could be using the existing script passing an specific version, it would break them if we change the arguments to latest, beta or rc

Can we not handle both cases?

@iancardosozup
Copy link
Contributor Author

if someone wants to pass latest or a specific version as a parameter they can use the existing script, no? i created install-beta-rc.sh to receive only rc or beta arguments. Should i make a change to use a specific beta/rc version?

I think that would be better change the existing script. This new script looks very similar with the current.

someone could be using the existing script passing an specific version, it would break them if we change the arguments to latest, beta or rc

Can we not handle both cases?

i think it will get much verbose, don't you?

@matheusalcantarazup
Copy link
Contributor

if someone wants to pass latest or a specific version as a parameter they can use the existing script, no? i created install-beta-rc.sh to receive only rc or beta arguments. Should i make a change to use a specific beta/rc version?

I think that would be better change the existing script. This new script looks very similar with the current.

someone could be using the existing script passing an specific version, it would break them if we change the arguments to latest, beta or rc

Can we not handle both cases?

i think it will get much verbose, don't you?

Maybe, but I think that is still better than duplicating the entire script. What do you guys think @nathanmartinszup @wiliansilvazup ?

@wiliansilvazup
Copy link
Contributor

I don't beleave duplicate script is the better case.
User can pass argument:

  • Version: This argument can be:
    • latest get latest and stable version
    • latest-beta get latest beta version published
    • latest-rc get latest release candidate version published
    • vX.X.X specific version user pinned for usage in your environment
      other content will return exit 1

@wiliansilvazup
Copy link
Contributor

I don't beleave duplicate script is the better case. User can pass argument:

  • Version: This argument can be:

    • latest get latest and stable version
    • latest-beta get latest beta version published
    • latest-rc get latest release candidate version published
    • vX.X.X specific version user pinned for usage in your environment
      other content will return exit 1

@iancardosozup beta and release candidate in this case can be used passing specific version too v2.7.0-beta.1 or v2.7.0-rc.1

@iancardosozup
Copy link
Contributor Author

iancardosozup commented Nov 29, 2021 •
edited
Loading

I don't beleave duplicate script is the better case. User can pass argument:

  • Version: This argument can be:

    • latest get latest and stable version
    • latest-beta get latest beta version published
    • latest-rc get latest release candidate version published
    • vX.X.X specific version user pinned for usage in your environment
      other content will return exit 1

@iancardosozup beta and release candidate in this case can be used passing specific version too v2.7.0-beta.1 or v2.7.0-rc.1

So i'll need to create a semver validation in bash script? i was thinking about and i could validate latest-beta and latest-rc
then call
curl -fsSL https://raw.githubusercontent.com/ZupIT/horusec/main/deployments/scripts/install.sh | bash -s $VERSION

i really don't think passing all parameters is a good practice, if someone wants to access beta and rc versions they or pass it by hand and we're creating another file to pass it as latest parameter

@iancardosozup iancardosozup force-pushed the feature/add-make-install-beta-and-rc branch from 624c7a8 to 067743b Compare November 29, 2021 19:52
@wiliansilvazup wiliansilvazup changed the title feature:makefile - add install-beta and install-rc makefile:feature - add install-beta and install-rc Nov 30, 2021
@iancardosozup iancardosozup force-pushed the feature/add-make-install-beta-and-rc branch from 067743b to c02e8fd Compare November 30, 2021 14:00
@iancardosozup iancardosozup changed the title makefile:feature - add install-beta and install-rc install-script:feature - add install-beta and install-rc Nov 30, 2021
@iancardosozup iancardosozup force-pushed the feature/add-make-install-beta-and-rc branch 4 times, most recently from b1f5ab3 to 0a338c8 Compare November 30, 2021 16:47
@iancardosozup iancardosozup self-assigned this Nov 30, 2021
nathanmartinszup
nathanmartinszup reviewed Nov 30, 2021
View reviewed changes
internal/services/horusec_api/horus_api.go Outdated Show resolved Hide resolved
internal/utils/copy/copy.go Outdated Show resolved Hide resolved
internal/services/formatters/python/safety/formatter.go Outdated Show resolved Hide resolved
@iancardosozup iancardosozup force-pushed the feature/add-make-install-beta-and-rc branch from 0a338c8 to 2e89936 Compare November 30, 2021 19:22
@nathanmartinszup
Copy link
Contributor

Rebase branch with main

@iancardosozup iancardosozup force-pushed the feature/add-make-install-beta-and-rc branch 4 times, most recently from 8165917 to 3206add Compare December 1, 2021 13:03
matheusalcantarazup
matheusalcantarazup suggested changes Dec 1, 2021
View reviewed changes
deployments/scripts/install.sh Outdated Show resolved Hide resolved
deployments/scripts/install.sh Outdated Show resolved Hide resolved
@iancardosozup iancardosozup force-pushed the feature/add-make-install-beta-and-rc branch from 3206add to d39bf9e Compare December 1, 2021 13:16
@iancardosozup
feature:script - add install-beta and install-rc to install script
29b373c 
now install.sh accepts latest-rc and latest-beta as input and checks if the argument is in a valid semantic version format (https://regex101.com/r/TKzfi9/1) besides latest format

Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br>
@iancardosozup iancardosozup force-pushed the feature/add-make-install-beta-and-rc branch from d39bf9e to 29b373c Compare December 1, 2021 13:25
@nathanmartinszup nathanmartinszup merged commit 4bc7690 into main Dec 1, 2021
@nathanmartinszup nathanmartinszup deleted the feature/add-make-install-beta-and-rc branch December 3, 2021 16:22
nathanmartinszup pushed a commit that referenced this pull request Dec 15, 2021
@iancardosozup @nathanmartinszup
feature:script - add install-beta and install-rc to install script (#827
86979da 
)

now install.sh accepts latest-rc and latest-beta as input and checks if the argument is in a valid semantic version format (https://regex101.com/r/TKzfi9/1) besides latest format

Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br>
(cherry picked from commit 4bc7690)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matheusalcantarazup matheusalcantarazup matheusalcantarazup approved these changes

@wiliansilvazup wiliansilvazup wiliansilvazup approved these changes

@nathanmartinszup nathanmartinszup nathanmartinszup approved these changes

@lucasbrunozup lucasbrunozup Awaiting requested review from lucasbrunozup

Assignees

@iancardosozup iancardosozup

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@iancardosozup @matheusalcantarazup @wiliansilvazup @nathanmartinszup