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

Ability to configure stage to only allow manual trigger on success of previous stage #6294

Closed
granddesigntech opened this issue May 16, 2019 · 13 comments

Comments

@granddesigntech
Copy link

commented May 16, 2019

Issue Type
  • Feature enhancement
Summary

I have stages in my pipeline that require resource with specific role to manually execute as a way of approval management.

However, manual trigger does not stop the manual trigger if the previous stage failed. This means that potential for human error is not alleviated by this automation whic is of course a major selling point to our stakeholders.

We would like to have the manual trigger configuration allow to specify whether manual trigger should be possible only upon success of previous stage or regardless of previous stage result.

Environment

NA - not a bug

Basic environment details

NA - not a bug

Additional Environment Details

NA - not a bug

Steps to Reproduce
  1. create pipeline with stage 1 hardcoded to fail
  2. add stage 2
  3. set stage 2 Type to Manual
  4. run pipeline
Expected Results

stage 2 is not manually triggerable

Actual Results

stage 2 is manually triggerable

Possible Fix

This feature enhancement request

Log snippets

NA - Not a bug

Code snippets/Screenshots

NA - Not a bug

Any other info

NA - Not a bug

@maheshp

This comment has been minimized.

Copy link
Member

commented May 24, 2019

With the manual trigger I am assuming the users would be taking an informed decision before triggering the stage.

Instead of introducing a configuration, does it make sense to change the UX by taking a confirmation before actually triggering the stage. IMO adding a configuration for this would lead to a configuration proliferation.

@arvindsv

This comment has been minimized.

Copy link
Member

commented May 24, 2019

I thought about that and then realized that it already has a confirmation. So, the original request then is a specific flag to turn it off. To disallow forcing it.

@maheshp

This comment has been minimized.

Copy link
Member

commented May 28, 2019

I thought about that and then realized that it already has a confirmation.

Yeah, I dint realize that.

@granddesigntech

This comment has been minimized.

Copy link
Author

commented May 30, 2019

With the manual trigger I am assuming the users would be taking an informed decision before triggering the stage.

This is for an enterprise customer in the finance industry and they have security requirements that are not legal to leave up to the good judgment of their personnel.

A little more proliferation of configuration enabling tighter security controls wouldn't go astray to be honest.

@maheshp

This comment has been minimized.

Copy link
Member

commented May 30, 2019

A little more proliferation of configuration enabling tighter security controls wouldn't go astray to be honest.

@granddesigntech fair enough! I understand your requirements better now.

@maheshp maheshp added this to the NextUp milestone May 30, 2019

@kritika-singh3

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2019

Introducing additional check if the approval type for stage run is manual by adding config attribute/tag, which when configured with success, will allow to manually trigger only on a successful run of the previous stage .
Introducing additional check to be performed for manual trigger of a stage if the attribute allowOnlyOnSuccess is set - the stage will be scheduled only if the previous stage resulted in success.
Updated docs link: https://docs.gocd.org/19.8.0/configuration/configuration_reference.html#approval

Currently, cruise-config.xml looks like:

<approval type="manual" />

which will get updated to:

  1. Updating manual and success to subtags
<approval>
  <manual allowOnlyOnSuccess="success" /> #or
  <manual allowOnlyOnSuccess="any" />  #or
  <success />
</approval>  
  1. Adding allowOnlyOnSuccess subtag to approval tag
<approval type="manual">
  <allowOnlyOnSuccess on="success" /> #or
  <allowOnlyOnSuccess on="any" />  #default
</approval>  

@gocd/godev WDYT?

@arvindsv

This comment has been minimized.

Copy link
Member

commented Jun 10, 2019

I feel like something like: <approval type="manual" allowOnlyOnSuccess="true" /> has the advantage of keeping all existing configs valid and any scripts like gomatic working.

@kritika-singh3

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2019

In the case of type="success", allowOnlyOnSuccess will make no sense, wouldn't it?

@arvindsv

This comment has been minimized.

Copy link
Member

commented Jun 10, 2019

Correct. It wouldn't. We'd need to set that to false, which will be the the default and so, it'll remove it from the config, is what I was thinking.

@kritika-singh3

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2019

Adding allowOnlyOnSuccess attribute to approval tag which will be utilized if type is set to manual.

TODO

@kritika-singh3

This comment has been minimized.

Copy link
Contributor

commented Jun 19, 2019

@arvindsv @gocd/godev,

Thoughts about:

  • which is preferred runOnlyOnSuccess or allowOnlyOnSuccess?
  • should we utilize this attribute for success approval as well?
    Meaning, in case a stage fails, we can force trigger the next stage. Should we use this trigger for
    that behavior as well instead of just manual triggers?
@arvindsv

This comment has been minimized.

Copy link
Member

commented Jun 19, 2019

Meaning, in case a stage fails, we can force trigger the next stage. Should we use this trigger for
that behavior as well instead of just manual triggers?

Yes, I think so.

which is preferred runOnlyOnSuccess or allowOnlyOnSuccess?

Since this is mostly about a user-intervention, I felt allow made sense.

@maheshp maheshp modified the milestones: NextUp, Release 19.7.0 Jul 16, 2019

@maheshp maheshp added this to In progress in 19.7.0 Jul 16, 2019

@maheshp maheshp removed this from In progress in 19.7.0 Jul 25, 2019

@maheshp maheshp added this to In progress in 19.8.0 Jul 25, 2019

@maheshp maheshp moved this from In progress to Done in 19.8.0 Sep 5, 2019

@rajiesh

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

Verified on 19.8.0 (9912-33898b3adaf72cedea1604882d3554fd586571a8)

@rajiesh rajiesh closed this Sep 5, 2019

@rajiesh rajiesh moved this from Done to QA Done in 19.8.0 Sep 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
6 participants
You can’t perform that action at this time.