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

Set env vars for device groups #4538

Open
4 tasks
robmarcer opened this issue Sep 23, 2024 · 14 comments
Open
4 tasks

Set env vars for device groups #4538

robmarcer opened this issue Sep 23, 2024 · 14 comments
Assignees
Labels
customer request requested by customer feature-request New feature or request that needs to be turned into Epic/Story details headline Something to highlight in the release
Milestone

Comments

@robmarcer
Copy link
Contributor

robmarcer commented Sep 23, 2024

Description

Allow env-vars to be set for a device group. Any new devices added to that group would take on those variables.

If a grouped device's evn-var is replaced for that specific device then that new value should be retained.

Which customers would this be available to

None

Have you provided an initial effort estimate for this issue?

I have provided an initial effort estimate

Tasks

  1. Steve-Mcl
  2. Steve-Mcl
@robmarcer robmarcer added feature-request New feature or request that needs to be turned into Epic/Story details needs-triage Needs looking at to decide what to do labels Sep 23, 2024
@robmarcer
Copy link
Contributor Author

@robmarcer robmarcer added the customer request requested by customer label Sep 23, 2024
@joepavitt
Copy link
Contributor

Thanks @robmarcer - we have a product planning meeting on Wed, so will schedule it then

@Steve-Mcl
Copy link
Contributor

For consideration:

This is a good example of why the concept of Tags were raised (and developed, but that work stalled - possibly a very outdated branch knocking around). e.g. A Tag containing "things" (in this case env vars) could be applied to device(s), instance(s), application(s) and device group(s). As more and more of this kind of requirement arises (like "we need to apply a specific theme to group 1" or "we want the editor path set to 'live' for our 'live' instances, etc), tags begin to make more sense.

@joepavitt
Copy link
Contributor

@Steve-Mcl we're not going to be pursuing tags at this stage. The scope would be too considerably, and I don't want to introduce even more concepts into FF when users already find the existing number of concepts overwhelming.

@joepavitt joepavitt added this to the 2.10 milestone Sep 25, 2024
@joepavitt joepavitt added headline Something to highlight in the release and removed needs-triage Needs looking at to decide what to do labels Sep 25, 2024
@knolleary
Copy link
Member

Want to keep this simple.

  • Add a settings property to DeviceGroup - type Text, storing JSON blob
  • Define env vars under { "env": ... } style property - to match Instance settings format
  • Update API endpoint to support updating of env vars

Important to keep api and data format consistent with Instance env settings.

@Steve-Mcl
Copy link
Contributor

@joepavitt , if the time for you to get wysiwyg stuff in order is greater or equal to this, I could pick up the backend part of this (and frontend if required)?

@joepavitt
Copy link
Contributor

I can handover WYSIWYG this afternoon, but you're likely to need to juggle both as they're not small tasks

@Steve-Mcl
Copy link
Contributor

Steve-Mcl commented Oct 16, 2024

  • Update API endpoint to support updating of env vars

inheritance rules

Any thoughts on the rules for when a device belonging to a group has its own env vars?

  1. merge - group env vars have precedence
  2. merge - device env vars have precedence
  3. replace device env vars with group env vars
    • and if the group has zero env vars, send none to device (regardless of device settings)?
    • or if group has zero env vars, send existing device vars?

My gut says "1. merge - group env vars have precedence" because the settings should proliferate from the parent group to child devices however this then makes it impossible to apply unique values (overrides) on the device, making "2. merge - device env vars have precedence" the more favourable / sensible option.

Of course, there is another option: we provide a merge rule option for each env var. This provides greater flexibility but it is "new ground.".

In the event of no feedback, I will implement #2

Updating devices:

  • currently, with application devices, when a devices own Env vars are updated + saved, the device is immediately instructed to restart so it can pick up new env values.
  • Would we want potentially hundreds of devices to restart because a group env var has changed?
    • From my own production env experience, this is undesirable since you may want to simply prepare changes ahead of time, to be later picked up by the devices at an appropriate window of time.

@joepavitt
Copy link
Contributor

Devices inherit from Group, but if Device has an Env Var defined, it overrides the inherited Group var

@joepavitt
Copy link
Contributor

Would we want potentially hundreds of devices to restart because a group env var has changed?

Add a warning, make it clear to the user that will happen when saving their env var changes

@knolleary
Copy link
Member

Regarding updating; agreed that isn't desirable; but then we have the matter of tracking pending changes with deployed changes - which complicates matters as well.

For 1st iteration, I'd (as Joe has just commented) trigger a restart across the devices with appropriate warning. It will be an opportunity for future improvement to have pending changes etc.

@Steve-Mcl
Copy link
Contributor

Steve-Mcl commented Oct 16, 2024

Would we want potentially hundreds of devices to restart because a group env var has changed?

Add a warning, make it clear to the user that will happen when saving their env var changes

This warning should be present at all times right (not just when saving)?

Reason: user may spend time entering nnn+ env vars, and upon pressing Save suddenly sees a notification stating this would (effectively) interrupt production (so cancels until a later time).

@Steve-Mcl
Copy link
Contributor

Steve-Mcl commented Oct 16, 2024

Question on RBACS:

Currently, a member can edit/update instance and device Env Vars. Is it envisioned the same permissions be applied to Device Group?

Considering current RBACS for device group create, update & delete are all Roles.Owner it doesnt feel right to have "Members" edit the env vars of a whole group of devices considering that could effectively and immediately interrupt production operations?

However, I will proceed on the assumption we are keeping the RBACS aligned (i.e. "Member" can edit) but feedback either way would be good for a sanity check.

@joepavitt
Copy link
Contributor

joepavitt commented Oct 16, 2024

Let's use common sense here, member's should not be able to modify env vars if it's going to then cause a roll out to 100's of production devices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer request requested by customer feature-request New feature or request that needs to be turned into Epic/Story details headline Something to highlight in the release
Projects
Status: Scheduled
Status: In Progress
Development

No branches or pull requests

4 participants