Thava Alagu wrote:
>
>
> I have attached the updated materials based on our discussions so far.
> Your comments are appreciated.
Thanks..
BTW I'd really like to see more comments (+ or -) from everyone in web
stack on this proposal.
Even if you don't plan to use phppgadmin, this is the first of this
kind of integrations ("an applicaton which drops a tree of *.php files
on the disk and some config somewhere") so the precedents established
by project will be used for other PHP apps later so the best time to
have an opinion is right here right now..
> SUNWphppgadmin-config package includes following 2 config files :
> ----------------------------------------------------------------------
> Config File Name Description
> ----------------------------------------------------------------------
> /etc/phppgadmin/config.inc.php Specifies parameters such as postgres
> server port number to connect.
>
> /etc/apache2/2.2/samples-conf.d/phppgadmin.conf
> Apache config file for phppgadmin.
> Mainly used to map URL space /phpgadmin
> into /usr/postgres/phppgadmin dir.
Something (name or content split) should change here. The inclusion of
/etc/apache2/2.2/samples-conf.d/phppgadmin.conf means the package is
tightly coupled with apache2 (and depends on it) but the other config
file under /etc/phppgadmin/ is not.
So if in some future I want to use phppgadmin with say lighttpd, I
must install SUNWphppgadmin-config but that brings in apache2 as well.
How about delivering:
SUNWphppgadmin: with all the /usr/* content
+ the web server-neutral config in /etc/phppgadmin/*
(Logistically it'd need to be SUNWphppgadmin + SUNWphppgadminr
for SFW integration, but for the sake of discussion I'm
thinking of the packages IPS user will see in practice.)
SUNWphppgadmin-apch22: with the apache2-specific glue [config]
In practice we'd document OpenSolaris users to simply
"pkg install SUNWphppgadmin-apch22" for using phppgadmin with built-in
apache2, that should have all the dependencies to bring everything in so
it "just works".
> /usr/postgres/phppgadmin directory content layout is given as below :
The interface table said this is all project private (makes sense!) so
don't include it in the main body of the ARC case because it is not
under review. You could just drop it entirely, or my preference is
move it to an appendix or a separate file in the case materials (while
it's not in the scope of the review it is often useful to have a sense
of what's where so I do like the file listings to be available
somewhere in the case materials).
> 4.11. Security Impact:
>
> By default, phppgadmin is not enabled to be active when apache
> webserver httpd service is enabled. The webserver should be
The above sentence would be more complete if you say "... when apache
webserver httpd service is enabled after installing the phppgadmin
packages."
That way it highlights the main point you're making there: that
installing these package isn't enough to enable it, you still have
to go and do additional manual steps.
I'd like to see some further discussion on this decision: why?
Presumably if I make the effort to "pkg install SUNWphppgadmin-apch22"
it must be because I want to use phppgadmin with apache2, no?
Why make me copy the config manually into conf.d? I can see the flip
side of the argument as well, but not sure it's entirely convincing
yet. Since the precedent set this case will be used by others (like
phpMyAdmin), I'd like to see it be convincing so we don't have to revisit
later.
> if it is not already running. The connection to postgres server
> is established by username and password.
BTW is the user prompted for that or is it stored in the config file?
If the latter, the file must be readable only the intended process
(presumably webservd here?)
> PostgreSQL Web Administration Tool phppgadmin(1M)
>
> An example apache httpd.conf file on solaris may include the
> following line to enable phppgadmin :
>
> Include /etc/apache2/2.2/samples-conf.d/phppgadmin.conf
That would work, but the intent is not to do includes directly
from samples-conf.d, they're just samples.
Instead: document that user can copy samples-conf.d/phppgadmin.conf
into conf.d/phppgadmin.conf and edit to taste (conf.d/* is included
automatically already so copying it there enables it, no editing of
httpd.conf is necessary so drop that part).
> For example, PHP version 5.2.4 on Solaris delivers pgsql.so
> module in following location:
> /usr/php5/5.2.4/modules/pgsql.so.
>
> The /etc/php5/5.2.4/conf.d/pgsql.ini configuration file for
Nothing wrong here right now, but it'd be better to avoid the full
paths with /5.2.4/ because that's about to change and it's too much of
a mainteanance burden to keep editing the man page (and - nobody will
remember to do it so customers will get incorrect info).
Your man page will be more future-proof if you make it a bit more
generic (refer to PHP5 and pgsql.so, for example, instead of exactly
5.2.4 and full paths).
--
Jyri J. Virkki - jyri.virkki at sun.com - Sun Microsystems
