Monviech / os-caddy-plugin

Caddy Plugin with GUI for OPNsense
Other
38 stars 0 forks source link

Add configuration option to log HTTP access to plain JSON files #90

Closed pmhausen closed 6 months ago

pmhausen commented 6 months ago

Feature discussed in issue #88

Monviech commented 6 months ago

Thanks I'll look at it tomorrow. ^^

pmhausen commented 6 months ago

mkdir -p creates the entire path, that's what -p means.

As for the default values, fine with that. I thought setting the default to 0 in the XML would ensure there already is one.

pmhausen commented 6 months ago

Did the defaults for lines 11 and 215 - what about the integer value for LogAccessPlainKeep?

Monviech commented 6 months ago

Oh you are right. It either needs a default set, or the roll_keep needs an if statement so it only appears if a value have been actually set in the GUI. I always hard coded them since I just used sensible defaults. Also an additional value that limits the size would be nice, since otherwise, the log files could grow without limit and fill the hdd up (maybe, I don't know how likely this is).

Maybe an additional roll by size, just to keep safe, would be good?

pmhausen commented 6 months ago

Oh, you do not need to set them at all. Caddy comes with sane defaults builtin:

roll_size 100MiB
roll_keep 10
roll_keep_for 90d

According to the docs roll_keep_for can override roll_keep so i thought this might be the only option one might want to change, e.g. for GDPR compliance. A short duration of logs in the order of 7-14 days is generally considered OK for technical and audit reasons. So I picket 10d as the default in the plugin.

As for the code - if I set a default in the model, e.g.

            <LogAccessPlain type="BooleanField">
                <Default>0</Default>
            </LogAccessPlain>

or

            <LogAccessPlainKeep type="IntegerField">
                <Default>10</Default>
                <MinimumValue>1</MinimumValue>
                <ValidationMessage>Please enter a valid number of 1 or larger.</ValidationMessage>
            </LogAccessPlainKeep>

I'll add a default of 10 to the template if that is what you prefer, anyway.

Kind regards

Monviech commented 6 months ago

Thank you. :)