flowfuse icon indicating copy to clipboard operation
flowfuse copied to clipboard

Set env vars for device groups

Open robmarcer opened this issue 1 year ago • 5 comments

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

robmarcer avatar Sep 23 '24 10:09 robmarcer

This has been requested by the following two enterprise customers

https://app-eu1.hubspot.com/contacts/26586079/record/0-2/8845845707

&

https://app-eu1.hubspot.com/contacts/26586079/record/0-2/7039008216

robmarcer avatar Sep 23 '24 10:09 robmarcer

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

joepavitt avatar Sep 23 '24 10:09 joepavitt

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.

Steve-Mcl avatar Sep 23 '24 10:09 Steve-Mcl

@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 avatar Sep 23 '24 10:09 joepavitt

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.

knolleary avatar Sep 25 '24 13:09 knolleary

@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)?

Steve-Mcl avatar Oct 08 '24 10:10 Steve-Mcl

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

joepavitt avatar Oct 08 '24 10:10 joepavitt

  • 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.

Steve-Mcl avatar Oct 16 '24 09:10 Steve-Mcl

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

joepavitt avatar Oct 16 '24 10:10 joepavitt

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

joepavitt avatar Oct 16 '24 10:10 joepavitt

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.

knolleary avatar Oct 16 '24 10:10 knolleary

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 avatar Oct 16 '24 10:10 Steve-Mcl

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.

Steve-Mcl avatar Oct 16 '24 10:10 Steve-Mcl

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.

joepavitt avatar Oct 16 '24 10:10 joepavitt

Status Update as of writing:

  • UI done - #4660 merged into API PR - tested on pre-staging
  • API done - #4658 merged - tested on prod
  • Docs - #4666 - in review
  • Changelog - in progress

Steve-Mcl avatar Oct 21 '24 14:10 Steve-Mcl

Status update

  • Main branches - merged & checked on prod
  • Docs - https://github.com/FlowFuse/flowfuse/pull/4666 - in review
  • Changelog - https://github.com/FlowFuse/website/pull/2690 - in review

Steve-Mcl avatar Oct 21 '24 16:10 Steve-Mcl

Hi, I saw that the UI is already available for the devices groups environments in FlowFuse, But It is a little bit different to the other pages. For example the settings page of a device.

At the purple like there should be a padding. At the yellow like the blue line below "settings" is disappeared when you click on "Environment". image

Thanks for the great work, it will be really helpful!

78wesley avatar Oct 22 '24 16:10 78wesley

@knolleary any reason this was shifted to 2.11 and not closed?

joepavitt avatar Oct 29 '24 10:10 joepavitt

@joepavitt because I did a bulk update of all the still-open 2.10 items to 2.11 on Friday. I didn't spot this item should have been closed.

knolleary avatar Oct 29 '24 10:10 knolleary