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

Variable preview is misleading when a variable is scoped to a role other than the one on the step. #6944

Closed
6 tasks done
donnybell opened this issue Jul 8, 2021 · 14 comments
Assignees
Labels
area/steps feature/variables kind/bug This issue represents a verified problem we are committed to solving tag/breaking-change The resolution of this issue introduced a deliberately breaking change

Comments

@donnybell
Copy link

donnybell commented Jul 8, 2021

Original report preserved for context, see the thread for the problem

Prerequisites

  • I have verified the problem exists in the latest version
  • I have searched open and closed issues to make sure it isn't already reported
  • I have written a descriptive issue title
  • I have linked the original source of this report
  • I have labelled the value stream (area/core, area/steps, ...)
  • I have tagged the issue appropriately (kind/bug, kind/enhancement, feature/ ...)

The bug

Variables are not honoring Step Role Scoping in certain situations

What I expected to happen

Variable evaluations should honor the scope specificity and take the Step Role into consideration first.
https://octopus.com/docs/projects/variables#Scopingvariables-Scopespecificity

Steps to reproduce

  1. Create a project with a "Run a Script" step that does write-host $variable1
    • Execution Location = Run on each deployment target
    • On Targets in Roles = Dev
  2. Create variable1 with the following values:
    • 1 scoped to Dev Environment
    • 2 scoped to Test Environment
    • #{variable2} scoped to Extra Role
  3. Create variable2 with the following values:
    • 3 scoped to Dev Environment and Extra Role
    • 4 scoped to Test Environment and Extra Role
  4. Create a Release and Deploy against a Target in the Dev Environment with Dev and Extra Roles
  5. Task Logs should reveal a 3

Screen capture

Step 1
image

Step 2 and 3
image

Step 4
image

Variable Preview
image

Task Log
image

Affected versions

Confirmed on 2021.1.7469

Workarounds

None

Links

Internal Link

@donnybell donnybell added kind/bug This issue represents a verified problem we are committed to solving feature/variables area/steps labels Jul 8, 2021
@droyad
Copy link
Contributor

droyad commented Jul 8, 2021

@donnybell What is the result you expect?

Variable evaluations should honor the scope specificity and take the Step Role into consideration first.

In your scenario the step role is Dev. However there are no variables scoped to the role Dev, therefore it picks the variable scoped to Extra

@droyad
Copy link
Contributor

droyad commented Jul 8, 2021

The ranking code is:

long score = 0;
score += Score(ScopeField.Private, 100000000000);
score += Score(ScopeField.User, 10000000000);
score += Score(ScopeField.ParentDeployment, 1000000001);
score += Score(ScopeField.Trigger, 1000000000);
score += Score(ScopeField.Action, 100000000);
score += Score(ScopeField.Machine, 10000000);
score += Score(ScopeField.TargetRole, 1000000); // Role as specified on the step
score += Score(ScopeField.Role, 100000); // Role of the deployment target/worker (AKA Machine)
score += Score(ScopeField.Tenant, 10001);
score += Score(ScopeField.TenantTag, 10000);
score += Score(ScopeField.Environment, 1000);
score += Score(ScopeField.Channel, 100);
score += Score(ScopeField.ProcessOwner, 10);
score += Score(ScopeField.Project, 1);
rank = score;

Environment = Dev
StepRole = Dev
Role = Dev, Extra

So the scores for variable1 are:

  • 1,001 -> 1 scoped to Dev Environment
  • 1 -> 2 scoped to Test Environment
  • 100,001 -> #{variable2} scoped to Extra Role

Despite the Extra role not being specified in the step, it is still considered. It is rule 4 in the scope specifity (more information.

@droyad
Copy link
Contributor

droyad commented Jul 8, 2021

The variable preview is misleading (internal reproduction)

When a specific deployment target is selected, it should take the target's roles into account:
image

If you select the Role filter, it shows the right thing (note the message says "for deployment targets with role other").

image

@droyad droyad changed the title Variables are not honoring Step Role Scoping in certain situations Variable preview is misleading when a variable is scoped to a role other than the one on the step. Jul 8, 2021
@droyad
Copy link
Contributor

droyad commented Jul 8, 2021

Proposed solution

  • The Role field in the preview is removed
  • The preview takes the target's roles into account

Alternate

  • The Role field is renamed to Target's Roles
  • The Role field allows multiple selections
  • The Role and Deployment Target fields are mutually exclusive (since what should we do if they conflict).
  • The preview takes the selected target's roles into account

@gurufordy
Copy link

Hi, this was my bug so thought I'd share my thoughts if that's ok.

Firstly there's two ways of previewing what would happen aren't there. The 'Preview' under Variables was not great as the scoping only allows single selection, so making it multi would of helped. I ended up using the 'All' under Variables and filtering the list down as it allowed multi selection.

With what you've put above as a solution, does it actually solve the issue as I can't see it say in the ranking code that it uses the TargetRole(s) of the Step, which should then obviously filter out the other variables. Not my code, so apologies if it does and I'm wrong! Re-reading it, is it the first bullet in 'Alternate'? I thought that was for the Preview not ranking code, so not sure :)

@droyad
Copy link
Contributor

droyad commented Jul 11, 2021

@gurufordy Re-reading the ranking code I can see the confusion. TargetRole in that context means the step's role (i.e target of the step). (Background: This is old old code and the only place this term is used. Deployment Targets used to be called Machines until workers were introduced.)

Paul's blog post does a good job of covering the reasons behind this quirk.

@gurufordy
Copy link

Thanks @droyad, thanks for the link and yeah that does read right now 👍

I'm not sure the Role and Deployment Target filters are mutually exclusive though are they? I assumed the Role is there in place of the Step Role that a deployment would have?

Also if most of the changes are around the preview, is there an actual fix for the deployment evaluation?

@droyad
Copy link
Contributor

droyad commented Jul 13, 2021

Role is there in place of the Step Role

The step role can be determined from the selected step

is there an actual fix for the deployment evaluation?

Given the scenario described in the first section of this issue, it is working as intended (and described in the docs/blog) I think.

@gurufordy
Copy link

Actually looking over the example and relating it back to my actual issue, you may be right that it's not an issue.

I guess it's a misunderstanding, but I thought having an extra scoping to a different role, would only use that value on a step with the matching role, i.e. Donny's example I expect it to evaluate to 1 as the scoping for value 3 is ignored due to not a matching role.

So in short the scoping of a variable does not always work on an exact match basis(?)

@droyad
Copy link
Contributor

droyad commented Jul 14, 2021

So in short the scoping of a variable does not always work on an exact match basis(?)

I would put it that scoping of a variables to roles is based on the target's roles and that the step role is only used as a tie breaker.

@gurufordy
Copy link

Would the tie breaker not be needed in this example though as without the step role it's got two different values for the variable?

Sorry to keep probing, but this is a little counter-intuitive to me. Are there any other scoping values that are not used explicitly like the step role?

@droyad
Copy link
Contributor

droyad commented Jul 22, 2021

In this scenario there is no tie to break.

The Tenant Tags scope would work in the same way. The tags on the tenant only are considered, not those on the step or any prior selection.

The way I think of it is that the scope is applied to the thing. i.e. the Environment, or the target or tenant.

Doing it this way does let you specify say a proxy value or subnet gateway to a group of targets independent of the step role.

@N-lson
Copy link

N-lson commented Aug 12, 2021

Release Note: Remove role input from variable preview, instead retrieve role scope based on the deployment target selected

@zentron zentron added the tag/breaking-change The resolution of this issue introduced a deliberately breaking change label Jan 5, 2022
@Octobob
Copy link
Member

Octobob commented Jan 18, 2022

🎉 The fix for this issue has been released in:

Release stream Release
2021.3 2021.3.1198
2022.1+ all releases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/steps feature/variables kind/bug This issue represents a verified problem we are committed to solving tag/breaking-change The resolution of this issue introduced a deliberately breaking change
Projects
None yet
Development

No branches or pull requests

6 participants