Issue #7059 has been updated by Daniel Pittman. Status changed from Unreviewed to Accepted Priority changed from Normal to Immediate Target version set to Statler Estimated time set to 24.00 Affected Puppet version set to development
Ah! Thank you for the cogent explanation; I now understand the scope of the problem. There is an unfortunate confluence of design decisions that make this extremely difficult to resolve, and once we get past the initial problem we will simply expose a second round of problems. The actual issue in this report is a previously encountered issue: we implemented inheritance for faces, but we did it in a naive way that resulted in the actions binding to their defining face (== class) rather than the class they are accessed through; they don't see options that were added below them. (I believe they will also be unable to see actions added below them, making some of the abstract superclass models impossible to implement at the moment, but this is less of a pressing issue.) Resolving that requires resolving the rebinding problem that also breaks the `Action#invoke` method, which is that we need to implement correct binding for actions and options to their derived class. Once that is done the option will be visible at the level of indirected actions. However, that will reveal the second part of the problem, which is that we have no model for overriding an inherited action. The `ca-location` option has to mutate global state, because our CA location is set for all code in the current instance, but the faces model tries to encourage us to use stateless code. We have currently worked around this in the subclass defined actions by setting, and then resetting, the value around the actual implementation of each call. If we had functional overriding of inherited actions, and the ability to call a superclass action, we could wrap, or replace, the inherited code and make this work. There are four concrete approaches to that side of things that I can see: First, we can stop being an indirector face and just become a normal face. We then have to duplicate the code that implements all the indirector actions and options into the certificate face, but we can at least have the correct public API. Internal ugliness, not public ugliness. Second, we can implement the ability to override a superclass action entirely. This, more or less, is a different mechanism for implementing the first option, and probably has no extra benefits. (Theoretically it gives us inherited options, so we don't reimplement that part, but a one line declaration is worth practically nothing.) Third, we can implement some sort of "around", of "before/after" mechanism for faces. One of these has already been mooted, adding "hooks" to options that execute code before and after each method is called, or perhaps around it. This would, assuming options correctly inherit, allow the subclass to add implicit behaviours to the superclass, and would resolve the problem. Casting that as the ability to decorate actions directly would let us open-code the same behaviour. That adds substantially more power to the system, which I suspect is a worse design decision that the limited power of before/after/around decoration in options, but either would count to solve this with path 3. Finally, we can ditch the stateless nature of the face entirely. By extending the system to allow us to globally change the CA location in the command line code, and just ignoring it entirely in the face, we can make it possible for this code to work during the transition period. That isn't a long term solution, just a short term work-around. My guess is that the best solution is to give limited action decoration capabilities to options, and use those. ---------------------------------------- Bug #7059: indirector action for the certificate face needs to take a ca-location options https://projects.puppetlabs.com/issues/7059 Author: Dan Bode Status: Accepted Priority: Immediate Assignee: Daniel Pittman Category: strings Target version: Statler Affected Puppet version: development Keywords: Branch: It fails with an exception b/c it does not accept this required option. -- You have received this notification because you have either subscribed to it, or are involved in it. To change your notification preferences, please click here: http://projects.puppetlabs.com/my/account -- You received this message because you are subscribed to the Google Groups "Puppet Bugs" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-bugs?hl=en.
