> Spring 3.x approach: Q: "Hey, there's this placeholder ${ssoSession.maxTimeToLive:3600} in ticketExpirationPolicies.xml, where's the value for that set?" A: "Oh look, the default is right there in-line, bet I could override that in cas.properties!"
Your Q/A thing highlighted two things for me: 1. We need better standards for property naming if we really name things without the units (i.e. maxTimeToLiveInSeconds) :-) 2. Having people read through all the Spring XML files rather than default.properties to locate the values that can be changed/paramaterized seems like more work for the deployer. I would think properly named placeholder names all located in one file would provide enough context as well as give us a single point of reference when questions come up. Do we want people to care which XML file the properties are in if they don't plan on overlaying those files? I think you and I are in agreement though that its time to get it right (even if we don't completely agree on what "right" is ;-)) Cheers, Scott On Tue, Jan 3, 2012 at 4:44 PM, William G. Thompson, Jr. <wgt...@gmail.com>wrote: > What started this discussion was a desired to improve maven overlay > based CAS deployments for the 3.5 release. The release strategy > states that in a minor release (i.e. 3.4.x -> CAS 3.5) "CAS maven > overlays may require minor to moderate changes". So, I tend to want > to get this right for 3.5 even if it requires some minor to moderate > changes to existing deployments. > > Generally, the first thing I do is externalize cas.properties with the > goal of having a single war file that can be deployed to multiple > tiers/nodes without further configuration (i.e. deploy/unpack the war, > find the right config files, edit/copy the old ones, etc). Having > some configuration parameters in cas.properties file is great, having > it in embedded in the war file makes it less convenient for > single-war-multiple-deployments. > > Overtime and many enterprise CAS deployments, I've also noticed that > logging configuration could also benefit from externalization, which > gave rise to CAS-1082, subsequent discussions including this one, and > the discovery that default values for bean configuration could be > in-lined like so: > > <property name="arguments"> > <list> > <value>${log4j.config.location:classpath:log4j.xml}</value> > <value>${log4j.refresh.interval:60000}</value> > </list> > </property> > > We also have the suggestion by Daniel Frett that a default.properties > be added and loaded before cas.properties to allow default settings to > be set for stock CAS and not cause issues for deployers when they > override cas.properties in their own maven overlay. > > I agree with Scott's comments regarding coming up with a general > strategy going forward (3.5+). So, here's a stab at an approach for > this minor but helpful improvement. > > CAS Configuration should follow the following principles: > > * Simple should be easy, complex should be possible > * Key config should be easily externalized so that a single war file > can easily be deployed to multiple tiers or nodes > * Consistent approach > * Generally seek to limit the number of files that need to be managed > in the overlay to make upgrading easier. > > A possible approach: > > * Push all defaults to the bean files where they are defined using the > Spring 3.x approach above and continue to use cas.properties as the > deployment override file for simple parameter configuration. Continue > to use deployerConfigContext for the rest of a typical deployer config > (simple should be easy) > > * Continue the use of bean xml file override for more complex behavior > configuration (complex is possible) > > * Create new property placeholders and defaults for bean properties > that could benefit from the new approach (e.g. SSO Session timeouts, > SLO on/off). (minimize the number of files that need to be managed in > the overlay) > > * Move the propertyFileConfigurer configuration block into > deployConfigContext.xml (minimize the number of files that need to be > managed in the overlay) > > Spring 3.x approach: > Q: "Hey, there's this placeholder ${ssoSession.maxTimeToLive:3600} in > ticketExpirationPolicies.xml, where's the value for that set?" > A: "Oh look, the default is right there in-line, bet I could override > that in cas.properties!" > > Bill > > > On Tue, Jan 3, 2012 at 11:05 AM, Scott Battaglia > <scott.battag...@gmail.com> wrote: > > > > On Tue, Jan 3, 2012 at 10:59 AM, Andrew Petro <ape...@unicon.net> wrote: > >> > >> I'm ambivalent about the value of this change to introduce an additional > >> default.properties to be parsed prior to cas.properties, which would > >> supersede default.properties' values. It feels like it's adding > complexity > >> without solving a problem worth solving. > > > > > > Minimizing upgrade pain and placing the defaults in a single location > when > > we realize something should be paramaterized is not worth solving? I'm > > confused. Especially since not having a mechanism like this in place > > couldn't add parameters except in major releases or if we spread the > > defaults throughout all the files (a la the Spring 3 syntax) > > > > > > > > > > > > > >> > >> > >> Again, this is: > >> > >> https://issues.jasig.org/browse/CAS-1084 > >> > >> and > >> > >> https://github.com/Jasig/cas/pull/23 > >> > >> I'm having trouble empathizing with the problem that this seems to be > >> trying to solve. Updating the cas.properties file on a CAS version > upgrade > >> seems like a totally reasonable burden on the upgrader. If CAS started > >> configuring TGT timeouts in cas.properties, I wouldn't regard it as too > much > >> to ask *upgrading* CAS adopters to notice the new properties in the > >> shipping-in-CAS cas.properties and either add these to their localized > >> cas.properties or delete their local cas.properties and re-fork from > the new > >> default. I don't see having to update a local cas.properties that > worked > >> with version 3.4 of CAS to provide the properties required by 3.5 of > CAS as > >> a problem at all, and I don't value sparing adopters that particular > pain on > >> upgrade. > >> > >> This is different from making new deployers set these values when they > >> first deploy CAS, since new deployers when first deploying CAS don't > have an > >> existing cas.properties file that would gum up getting the properties > and > >> values in the cas.properties shipping in CAS. I would have concern > about > >> CAS shipping with properties files that don't work. That's different > from > >> CAS shipping with a cas.properties that does work but worrying that some > >> adopters won't use that working cas.properties. > >> > >> In my experience, updating the local cas.properties file on an upgrade > to > >> include added properties just hasn't felt anything like a real problem, > just > >> a reasonable upgrade practices checklist item. On balance, I'd probably > >> rather have the fail-init-on-unfulfilled-placeholder behavior than the > >> missing-property-is-masked-by-default.properties behavior. > >> > >> > >> Adding default.properties feels like it's adding some complexity. > >> > >> Currently: > >> > >> Q: "Hey, there's this placeholder > >> ${cas.securityContext.casProcessingFilterEntryPoint.loginUrl} in > >> cas-servlet.xml, where's the value for that set?" > >> A: In the properties file set in propertyFileConfigurer.xml, which by > >> default is /WEB-INF/cas.properties . > >> > >> After this change > >> > >> Q: "Hey, there's this placeholder > >> ${cas.securityContext.casProcessingFilterEntryPoint.loginUrl} in > >> cas-servlet.xml, where's the value for that set?" > >> A: Well, it depends. in propertyFileConfigurer.xml, there's a list of > >> properties files, which by default is /WEB-INF/default.properties and > >> /WEB-INF/cas.properties. The last-parsed value wins. So, if this > property > >> is in cas.properties, that's where it's set. But if it's not in > >> cas.properties, then it's the value in default.properties. > >> > >> > >> It's not the end of the world, but the latter felt harder to explain, > and > >> the former felt simpler. > >> > >> > >> Currently, if I fat finger a property name in a local cas.properties, I > >> notice. Under the proposed change, the fat fingering is masked by a > default > >> value in default.properties. > >> > >> Will CAS upgrading deployers be more grateful that we spared them having > >> to update their cas.properties files on upgrades, or will they be more > >> grateful for missing cas.properties properties continuing to fail fast? > >> It's not clear to me that allowing subsets rather than complete sets of > >> properties in cas.properties files is worth losing the > >> fail-fast-on-missing-properties feature. Would deployers rather have > just > >> one properties file to worry about, or would they rather have two and > >> understand what it means for properties to be in which and not the > other? > >> It's not clear to me that the complexity is worth it. > >> > >> > >> I think I'm -0 for this change, but I don't think it's very important > and > >> I'll happily help upgrading adopters to understand the > >> cascading-properties-files approach if CAS implements it. > >> > >> Andrew > >> > >> > >> > >> > >> > >> On Jan 3, 2012, at 8:18 AM, Scott Battaglia wrote: > >> > >> > I'm not sure I agree with forcing you to add something. If we've > moved > >> > a formerly hard-coded property to now being configurable, you > shouldn't have > >> > to do anything. If its a new value, we should have a sensible default > >> > without requiring you to choose one (we don't make people set the TGT > >> > timeouts when they first deploy CAS). > >> > > >> > > >> > On Tue, Jan 3, 2012 at 8:14 AM, Marvin Addison > >> > <marvin.addi...@gmail.com> wrote: > >> > I'm really ambivalent about this approach. On the one hand it may > >> > ease the burden of upgrades when new properties are inevitably added. > >> > On the other hand it may facilitate upgrades inheriting undesirable > >> > behavior by default. I personally find it valuable for a deploy to > >> > break due to a new property missing from out custom cas.properties > >> > file, which forces me to review the change and consider whether the > >> > default is in fact desirable. > >> > > >> > M > >> > > >> > -- > >> > You are currently subscribed to cas-dev@lists.jasig.org as: > >> > scott.battag...@gmail.com > >> > To unsubscribe, change settings or access archives, see > >> > http://www.ja-sig.org/wiki/display/JSG/cas-dev > >> > > >> > -- > >> > You are currently subscribed to cas-dev@lists.jasig.org as: > >> > ape...@unicon.net > >> > To unsubscribe, change settings or access archives, see > >> > http://www.ja-sig.org/wiki/display/JSG/cas-dev > >> > >> > >> -- > >> You are currently subscribed to cas-dev@lists.jasig.org as: > >> scott.battag...@gmail.com > >> To unsubscribe, change settings or access archives, see > >> http://www.ja-sig.org/wiki/display/JSG/cas-dev > >> > > > > -- > > You are currently subscribed to cas-dev@lists.jasig.org as: > wgt...@gmail.com > > To unsubscribe, change settings or access archives, see > > http://www.ja-sig.org/wiki/display/JSG/cas-dev > > -- > You are currently subscribed to cas-dev@lists.jasig.org as: > scott.battag...@gmail.com > To unsubscribe, change settings or access archives, see > http://www.ja-sig.org/wiki/display/JSG/cas-dev > > -- You are currently subscribed to cas-dev@lists.jasig.org as: arch...@mail-archive.com To unsubscribe, change settings or access archives, see http://www.ja-sig.org/wiki/display/JSG/cas-dev