generated from AMRC-FactoryPlus/acs-template
-
Notifications
You must be signed in to change notification settings - Fork 6
Load Auth classes into the ConfigDB #385
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This requires R2 classes, which for now we have to create via a v1 dump by relying on load order. The v2 dump format needs to handle this better.
We mustn't create it here, this causes rank conflicts.
We need to be able to create (R1) classes via the `objects` key, which v1 dumps do not allow. Eventually v2 dumps want to be able to resolve loaded objects in any order; for now we are relying on the load order, which means relying on key order preservation in JS and YAML. Version 2 dumps will always be authoritative about the information they contain. There will not be the option to choose not to overwrite existing information. The /v1/load endpoint on the ConfigDB will continue to accept the parameter for compatibility; the new /load endpoint will not.
Change the `objects` key to an object-of-objects so we can give more detail about object creation. We will need this to specify subclass and membership information. For now just use this to accept a `name` key which we patch into _General info_. There is too much duplication in the existing dumps just setting names.
The old v1/load endpoint will deliberately only accept version 1 dumps.
* Update v2 dumps to use the new format. * Promote _Permission group_ to rank 2. Its members are groups of permissions. We can't set up subclass relations from dump files yet.
We can't re-rank objects yet, so make sure _Permission group_ is not created by a v1 dump. I'm not updating the main dump files to v2; they want to be migrated to service-setup instead. Move the git service dumps over entirely.
This means loading the schema file from the acs-configdb directory.
I don't like to do this but I don't see any option for now. The git server is crashing because it doesn't have permission to register itself with the Directory, and this is preventing service-setup from running.
I am having problems with service-setup not running because Pods won't start cleanly.
There is no reason to ever stop trying. We cannot say 'don't stop', this can only be achieved via `podFailurePolicy` which requires `restartPolicy: Never` on the Pod, and will create a new Pod for every new attempt. This will re-run all the containers, including those that succeeded. We may need to revisit this list-of-initContainers idea. Perhaps multiple Jobs would be better? But then they will need to wait for each other as appropriate. With non-atomic services just trying and failing may leave a mess.
The design here is: * The principal of an ACE must be a member of either _Principal_ or _Principal group_; otherwise the ACE is ignored. Similarly the permission. * We only expand groups for princ/perm where appropriate. * Targets are harder to handle; we need to extend the ACE data model to indicate whether a target is a group or an individual. There is no choice as we do not know what rank the target should be. The target-is-group flag needs to default to false; service-setup creates some group-target ACEs but these should probably be moved into a dump. Migration of existing non-ACS ACEs will need careful consideration, and probably a major version rev.
* Ensure subclass and member changes respect ranks. * Ensure an object is not removed from its primary class. * Ensure a class is not left with no superclass (except rank classes). * Remove redundant direct subclass relations.
In some places there are ConfigDB classes and Auth groups which fulfil the same role, for example _Edge Agent_ vs _Cell Gateway_. That particular case I'm not entirely sure how to handle; it may be best in practice to drop the existing _Edge Agent_ and replace it with _Cell Gateway_. The _Soft Gateway_ class has not been used in practice since v1, so _Edge Agent_ and _Cell Gateway_ are in fact the same class.
This uses dump functionality which isn't implemented yet.
The consistency checking depends critically on the existing objects in the database so it's going to be easiest to just get the data into the database and process it there. This only handle the object definitions. The configs can still be loaded separately. Create an SQL function to validate our invariants where they can't be checked with FKs. We should probably call this at startup.
That is, allow objects to be indirect members, only, of their primary class. This avoids the need for a large number of redundant direct memberships to satisfy primary class requirements.
This performs correct (order-independant) loading and resets all ranks correctly. Adjust the dump schema to accept the new keys. Adjust the SQL to update the GI names; there's no point doing this separately once we've got them into the DB.
Some revision of the structure was needed. This omits the definitions of the composite permissions, as these are enumerated classes (they have a fixed known list of members) which we don't support yet in v2 dumps. I have included REQUIRE comments to indicate deps between the dumps. With the current arrangements it isn't possible to make a dump entirely self-contained; the design is that we rely on existing objects to determine the ranks of the new objects. These comments are currently ignored by service-setup.
Each dump file can contain a (single) REQUIRE comment specifying dumps which must be loaded first. Load dumps in the correct order.
When the rank of an object changes we need to handle existing objects. Currently the design is: * All ranks are reset based on primary class. * Parent relations from existing objects which conflict with the new ranks will raise an error. * Existing objects promoted from individual to class are subclassed directly under the appropriate rank superclass. I'm not sure the second of these is the best behaviour, but I think it's safest for now. Silently removing the invalid relations would be the other possibility, which is less likely to cause an ACS upgrade to fail but more likely to lose something important.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The new ConfigDB dumps are not self-contained, as we can't randomly create classes when we don't know what rank they should be. This is handled with
REQUIREcomments in the dumps and code in service-setup to sort them appropriately.This does not attempt to migrate existing Auth group members into members of the equivalent ConfigDB class. That will need more thought.