Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Log the filenames agency users upload to the transition-logs server #136

Merged
merged 3 commits into from

7 participants

@issyl0
Collaborator
  • Bump the version of attachmentgenie/ssh to be able to do this, under the instruction of @dcarley.
  • Add the extra arguments to the sftp subsystem line to hieradata/common.yml. Thanks to @ajlanghorn and @mattbostock for getting me out of a rabbit hole of which file to edit.
  • Change the default destination for these new logs from syslog to sftp-server.log to avoid cluttering syslog.
@issyl0 issyl0 Log the filenames agency users upload to the transition-logs server
- Bump the version of attachmentgenie/ssh to be able to do this,
  under the instruction of @dcarley.
- Add the extra arguments to the `sftp subsystem` line to
  hieradata/common.yml. Thanks to @ajlanghorn and @mattbostock for
  getting me out of a rabbit hole of which file to edit.
- Change the default destination for these new logs from syslog to
  sftp-server.log to avoid cluttering syslog.
8028852
@issyl0
Collaborator

I'd like to limit this to only where it's currently useful - the transition-logs server. I'm in too deep with Puppet and have no idea how to do it though.

@jennyd jennyd commented on the diff
hieradata/common.yaml
@@ -101,3 +101,5 @@ cdn_logs::cert: |
YMYXvOaM6iswgdQGA1UdIwSBzDCByYAUCLsht8znHwAgdlpkYMYXvOaM6iuhgaWk
gaIwgZ8xCzAJBgNVBAYTAkdCMQ8wDQYDVQQIEwZMb25kb24xDzANBgNVBAcTBkxv
bmRvbjEjMCEGA1UEChMaR292ZXJubWVudCBEaWdpdGFsIFNlcnZpY2UxJDAiBgNV
+
@jennyd Owner
jennyd added a note

I would expect this to need an -----END CERTIFICATE----- line here, since it's no longer the end of the file.

@ajlanghorn Owner

+1 to @jennyd; it also looks like the cert might have been truncated unintentionally? It seems shorter than I'd expect.

@jamiecobbett Owner

This was changed (truncated?) by @dcarley here: 9a0efa6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jennyd
Owner

This will only affect transition-logs-1, because the cdn_logs module is only used there and not on the other CI machines.

@jennyd jennyd commented on the diff
modules/cdn_logs/manifests/init.pp
@@ -44,6 +44,13 @@
],
}
+ rsyslog::snippet { 'transition_logs_sftp':
@jennyd Owner
jennyd added a note

It's a bit confusing to have this config in this module, since we only want to log the file transfers made by agency users and not anything related to our CDN logs, which this module otherwise handles. Does this need to be in cdn_logs, or could it be in the transition_logs manifest which handles the rest of the agency user setup?

@issyl0 Collaborator
issyl0 added a note

There are already a few lines of rsyslog settings in the transition_logs manifest, and if I add the lines above to the transition_logs manifest, it errors:

Error: Could not apply complete catalog: Found 1 dependency cycle:
(File[/etc/rsyslog.d/transition_logs_sftp.conf] => Rsyslog::Snippet[transition_logs_sftp] => Class[Ci_environment::Transition_logs] => Rsyslog::Snippet[transition_logs_sftp] => File[/etc/rsyslog.d/transition_logs_sftp.conf])

@jamiecobbett Owner

@dcarley doesn't think this will be a problem, but the best way to find out is to deploy it and tail the fastly log and eg /var/log/syslog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ajlanghorn ajlanghorn commented on the diff
Puppetfile
@@ -7,7 +7,7 @@ mod 'puppetlabs/stdlib', '~> 4.0'
mod 'saz/dnsmasq', '1.0.1'
mod 'pdxcat/collectd', '~> 0.0'
mod 'attachmentgenie/ufw', '1.1.0'
-mod 'attachmentgenie/ssh', '1.1.1'
+mod 'attachmentgenie/ssh', '1.2.1'
@ajlanghorn Owner

I tend to use '~>' here in order to ensure that you have at least the version specified. For instance, '~> 1.2.1' would specify to ensure that, when librarian-puppet is run, it gets the latest version and installs that as long as the version number =>1.2.1. What say thee, @dcarley?

@mattbostock Owner

@ajlanghorn I remember now that the advice to pin to a specific version was for general GDS Ruby development, so not sure if the same applies to Puppet. I can't find the link unfortunately, it was in a Github wiki somewhere.

@mattbostock Owner

Thanks @alexmuller! I was looking at the Puppet styleguide... facepalm

It's near the top of this page.

@ajlanghorn Owner

Thanks @alexmuller. What's the verdict on doing the same in Puppet?

@alexmuller Owner

I think we should, but I am not the $LOCAL_PUPPET_EXPERT.

@issyl0 Collaborator
issyl0 added a note

Was there a decision on this? I'm tempted to do ~>.

@ajlanghorn Owner

~> is awesome. Perhaps resident Puppet guru, @dcarley, would be a good person to advise further.

@alext Owner
alext added a note

~> is great as long as you trust people to follow Semantic Versioning correctly, which isn't always the case.

@ajlanghorn Owner

@alext Ah, yeah, hadn't considered that. If someone releases a major as a minor etc., then it's a bit iffy. It's probably okay to use ~> on alphagov stuff; I think (though I can't remember) there's a way to say only update to the version most recent in this major/minor build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
hieradata/common.yaml
@@ -101,3 +101,5 @@ cdn_logs::cert: |
YMYXvOaM6iswgdQGA1UdIwSBzDCByYAUCLsht8znHwAgdlpkYMYXvOaM6iuhgaWk
gaIwgZ8xCzAJBgNVBAYTAkdCMQ8wDQYDVQQIEwZMb25kb24xDzANBgNVBAcTBkxv
bmRvbjEjMCEGA1UEChMaR292ZXJubWVudCBEaWdpdGFsIFNlcnZpY2UxJDAiBgNV
+
+ssh::server::subsystem_sftp: /usr/lib/openssh/sftp-server -f LOCAL7 -l VERBOSE
@mattbostock Owner

Looking at this with fresh eyes (good morning), I've realised that by putting this line in the common.yaml file, this setting for SFTP will apply to all machines using this Puppet repo.

For this to apply only to a specific machine, you would need to include this line in a YAML file specific to that machine. Looking in the hiera.yml file, this would be named along the lines of role.%{::machine_role}.yaml or role.%{::machine_role}.%{::machine_id}.yaml. Feel free to give me a shout if you're unsure about this.

@issyl0 Collaborator
issyl0 added a note

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@issyl0
Collaborator

I'm off ill today and don't have my work laptop here with me. I'll fix these issues up tomorrow if no-one gets to them first. Thanks!

@jamiecobbett

It looks to me like all the feedback has been resolved - is it good to merge?

@jamiecobbett jamiecobbett merged commit c10fe37 into from
@jamiecobbett jamiecobbett deleted the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 11, 2014
  1. @issyl0

    Log the filenames agency users upload to the transition-logs server

    issyl0 authored
    - Bump the version of attachmentgenie/ssh to be able to do this,
      under the instruction of @dcarley.
    - Add the extra arguments to the `sftp subsystem` line to
      hieradata/common.yml. Thanks to @ajlanghorn and @mattbostock for
      getting me out of a rabbit hole of which file to edit.
    - Change the default destination for these new logs from syslog to
      sftp-server.log to avoid cluttering syslog.
  2. @issyl0
Commits on Mar 17, 2014
  1. @issyl0
This page is out of date. Refresh to see the latest.
View
2  Puppetfile
@@ -7,7 +7,7 @@ mod 'puppetlabs/stdlib', '~> 4.0'
mod 'saz/dnsmasq', '1.0.1'
mod 'pdxcat/collectd', '~> 0.0'
mod 'attachmentgenie/ufw', '1.1.0'
-mod 'attachmentgenie/ssh', '1.1.1'
+mod 'attachmentgenie/ssh', '1.2.1'
@ajlanghorn Owner

I tend to use '~>' here in order to ensure that you have at least the version specified. For instance, '~> 1.2.1' would specify to ensure that, when librarian-puppet is run, it gets the latest version and installs that as long as the version number =>1.2.1. What say thee, @dcarley?

@mattbostock Owner

@ajlanghorn I remember now that the advice to pin to a specific version was for general GDS Ruby development, so not sure if the same applies to Puppet. I can't find the link unfortunately, it was in a Github wiki somewhere.

@mattbostock Owner

Thanks @alexmuller! I was looking at the Puppet styleguide... facepalm

It's near the top of this page.

@ajlanghorn Owner

Thanks @alexmuller. What's the verdict on doing the same in Puppet?

@alexmuller Owner

I think we should, but I am not the $LOCAL_PUPPET_EXPERT.

@issyl0 Collaborator
issyl0 added a note

Was there a decision on this? I'm tempted to do ~>.

@ajlanghorn Owner

~> is awesome. Perhaps resident Puppet guru, @dcarley, would be a good person to advise further.

@alext Owner
alext added a note

~> is great as long as you trust people to follow Semantic Versioning correctly, which isn't always the case.

@ajlanghorn Owner

@alext Ah, yeah, hadn't considered that. If someone releases a major as a minor etc., then it's a bit iffy. It's probably okay to use ~> on alphagov stuff; I think (though I can't remember) there's a way to say only update to the version most recent in this major/minor build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
mod 'puppetlabs/apt', '~> 1.3.0'
mod 'puppetlabs/git', '0.0.2'
mod 'puppetlabs/mysql', '0.9.0'
View
4 Puppetfile.lock
@@ -8,7 +8,7 @@ FORGE
specs:
attachmentgenie/locales (1.0.2)
puppetlabs/stdlib (>= 2.2.1)
- attachmentgenie/ssh (1.1.1)
+ attachmentgenie/ssh (1.2.1)
puppetlabs/stdlib (>= 2.2.1)
attachmentgenie/ufw (1.1.0)
puppetlabs/stdlib (>= 2.2.1)
@@ -118,7 +118,7 @@ GIT
DEPENDENCIES
attachmentgenie/locales (= 1.0.2)
- attachmentgenie/ssh (= 1.1.1)
+ attachmentgenie/ssh (= 1.2.1)
attachmentgenie/ufw (= 1.1.0)
clamav (>= 0)
cpanm (>= 0)
View
1  hieradata/common.yaml
@@ -101,3 +101,4 @@ cdn_logs::cert: |
YMYXvOaM6iswgdQGA1UdIwSBzDCByYAUCLsht8znHwAgdlpkYMYXvOaM6iuhgaWk
gaIwgZ8xCzAJBgNVBAYTAkdCMQ8wDQYDVQQIEwZMb25kb24xDzANBgNVBAcTBkxv
bmRvbjEjMCEGA1UEChMaR292ZXJubWVudCBEaWdpdGFsIFNlcnZpY2UxJDAiBgNV
+
@jennyd Owner
jennyd added a note

I would expect this to need an -----END CERTIFICATE----- line here, since it's no longer the end of the file.

@ajlanghorn Owner

+1 to @jennyd; it also looks like the cert might have been truncated unintentionally? It seems shorter than I'd expect.

@jamiecobbett Owner

This was changed (truncated?) by @dcarley here: 9a0efa6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
View
2  hieradata/role.transition-logs.yaml
@@ -5,6 +5,8 @@ classes:
- ci_environment::transition_logs
- ci_environment::firewall_config::transition_logs
+ssh::server::subsystem_sftp: /usr/lib/openssh/sftp-server -f LOCAL7 -l VERBOSE
+
ci_environment::transition_logs::rssh_users:
mhra:
comment: MHRA
View
7 modules/cdn_logs/manifests/init.pp
@@ -44,6 +44,13 @@
],
}
+ rsyslog::snippet { 'transition_logs_sftp':
@jennyd Owner
jennyd added a note

It's a bit confusing to have this config in this module, since we only want to log the file transfers made by agency users and not anything related to our CDN logs, which this module otherwise handles. Does this need to be in cdn_logs, or could it be in the transition_logs manifest which handles the rest of the agency user setup?

@issyl0 Collaborator
issyl0 added a note

There are already a few lines of rsyslog settings in the transition_logs manifest, and if I add the lines above to the transition_logs manifest, it errors:

Error: Could not apply complete catalog: Found 1 dependency cycle:
(File[/etc/rsyslog.d/transition_logs_sftp.conf] => Rsyslog::Snippet[transition_logs_sftp] => Class[Ci_environment::Transition_logs] => Rsyslog::Snippet[transition_logs_sftp] => File[/etc/rsyslog.d/transition_logs_sftp.conf])

@jamiecobbett Owner

@dcarley doesn't think this will be a problem, but the best way to find out is to deploy it and tail the fastly log and eg /var/log/syslog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ content => 'local7.* /var/log/sftp-server.log',
+ require => [
+ Class['ci_environment::transition_logs']
+ ]
+ }
+
ufw::allow { 'rsyslog_cdn_logs':
port => $port,
ip => 'any',
Something went wrong with that request. Please try again.