Tails

#1 Updated by intrigeri 2015-03-31 09:57:31

• Assignee set to kytv
• QA Check set to Info Needed

Kill Your TV, please specify what exactly is needed, and then reassign to bertagaz or I. Thanks!

#2 Updated by intrigeri 2015-03-31 09:58:04

• related to Feature #6308: Write tests for SSH added

#3 Updated by intrigeri 2015-03-31 09:58:21

• blocks #8538 added

#4 Updated by intrigeri 2015-03-31 09:59:17

• related to Feature #8920: Have and use a repo shared between core developers for storing test suite secrets added

#5 Updated by anonym 2015-03-31 14:14:05

• Assignee changed from kytv to bertagaz
• QA Check changed from Info Needed to Dev Needed

We need an SSH server available on the open internet (in the future we could also test it as a hidden service) with PubkeyAuthentication and the SFTP subsystem enabled, everything else can be locked down for security. If it helps, a non-standard port can be used.

FWIW, these are the details we later will need in the test suite’s local config:

SSH:
  username: "user"
  hostname: "some.host.org"
  port: 22222
  public_key: "ssh-rsa [...] user@another.host.org"
  private_key: |
    -----BEGIN RSA PRIVATE KEY-----
    [...]
    -----END RSA PRIVATE KEY-----

#6 Updated by intrigeri 2015-04-03 07:48:01

• Assignee changed from bertagaz to intrigeri

Thanks!

#8 Updated by intrigeri 2015-04-18 09:47:20

• Status changed from Confirmed to In Progress
• % Done changed from 0 to 10

#9 Updated by intrigeri 2015-04-19 07:40:32

• Feature Branch set to test/9142-ssh-falselogin

#10 Updated by intrigeri 2015-04-19 07:55:55

• blocked by Bug #9263: Split SSH and SFTP credentials in test suite configuration added

#11 Updated by intrigeri 2015-04-19 07:57:38

• % Done changed from 10 to 20

The infra and test suite adjustements are ready for SSH, and work fine. For SFTP, we’re blocked by Bug #9263 (my plan is to have the SSH account’s login shell to be falselogin, as already implemented, and another SFTP-only account for SFTP).

#12 Updated by intrigeri 2015-04-29 10:05:27

• Assignee changed from intrigeri to anonym
• % Done changed from 20 to 50
• QA Check changed from Dev Needed to Ready for QA

Done, works for me with the secrets that I’ve pushed to the repo created with Feature #9221 (that’s left to document). The branch has a minor UX improvement as its sole change.

#14 Updated by anonym 2015-05-05 14:48:47

• Assignee changed from anonym to intrigeri
• QA Check changed from Ready for QA to Info Needed

intrigeri wrote:
> Please reassign to bertagaz after merging, so that he can have a look at the Puppet side of things.

I’m just throwing and idea out there: why not clone the repo directly as features/config/local.d? Personal configuration files could be handled in your local branch, and you just keep on merging in origin/master. One could then keep even track of different sets of personal configuration files in branches, FWIW. Ìf we go the features/config/shared-secrets path we any way have to handle the problem of keeping the symlinks from local.d into it up-to-date, so I don’t see it making things simple, really.

#18 Updated by intrigeri 2015-05-06 20:28:33

• Assignee changed from intrigeri to anonym

#19 Updated by anonym 2015-05-08 09:33:48

• Assignee changed from anonym to intrigeri

intrigeri wrote:
> > I’m just throwing and idea out there: why not clone the repo directly as features/config/local.d? Personal configuration files could be handled in your local branch, and you just keep on merging in origin/master.
>
> This would work, and I’m not strongly opposed to it, but I don’t like it: I would find it confusing to have a directory called local.d be a clone of a remote Git repo.
>
> > Ìf we go the features/config/shared-secrets path we any way have to handle the problem of keeping the symlinks from local.d into it up-to-date, so I don’t see it making things simple, really.
>
> Right, this will quickly become painful (e.g. we’ll have to automate that for Jenkins). How hard would it be to just load stuff from each directory called features/config/*.d, in lexical order? Then local.d and shared-secrets.d become merely a convention, we can gitignore all these *.d directories, and anyone does what they want in features/config/.

It’s super simple, and I prefer this to everything else proposed here. All that would be needed is to change the glob to features/config/*.d/*.yml, but looking at the surrounding code I’d also clean up a bit too:

--- a/.gitignore
+++ b/.gitignore
@@ -52,4 +52,4 @@

 # The test suite's local configuration files
 /features/config/local.yml
-/features/config/local.d/
+/features/config/*.d/
diff --git a/features/support/config.rb b/features/support/config.rb
index ea5774a..9612f44 100644
--- a/features/support/config.rb
+++ b/features/support/config.rb
@@ -5,13 +5,14 @@ require "#{Dir.pwd}/features/support/helpers/misc_helpers.rb"
 # These files deal with options like some of the settings passed
 # to the `run_test_suite` script, and "secrets" like credentials
 # (passwords, SSH keys) to be used in tests.
-DEFAULTS_CONFIG_FILE = "#{Dir.pwd}/features/config/defaults.yml"
-LOCAL_CONFIG_FILE = "#{Dir.pwd}/features/config/local.yml"
-LOCAL_CONFIGS_DIR = "#{Dir.pwd}/features/config/local.d"
+CONFIG_DIR = "#{Dir.pwd}/features/config"
+DEFAULTS_CONFIG_FILE = "#{CONFIG_DIR}/defaults.yml"
+LOCAL_CONFIG_FILE = "#{CONFIG_DIR}/local.yml"
+LOCAL_CONFIG_DIRS_FILES_GLOB = "#{CONFIG_DIR}/*.d/*.yml"

 assert File.exists?(DEFAULTS_CONFIG_FILE)
 $config = YAML.load(File.read(DEFAULTS_CONFIG_FILE))
-config_files = Dir.glob("#{LOCAL_CONFIGS_DIR}/*.yml").sort
+config_files = Dir.glob(LOCAL_CONFIG_DIRS_FILES_GLOB).sort
 config_files.insert(0, LOCAL_CONFIG_FILE) if File.exists?(LOCAL_CONFIG_FILE)
 config_files.each do |config_file|
   yaml_struct = YAML.load(File.read(config_file)) || Hash.new

So should we go this way instead?

#20 Updated by intrigeri 2015-05-08 11:06:41

• QA Check changed from Info Needed to Dev Needed

>> How hard would it be to just load stuff from each directory called features/config/*.d, in lexical order? […]

> It’s super simple, and I prefer this to everything else proposed here. […]

Awesome, thanks for looking into it!

> So should we go this way instead?

Definitely. I’ll apply your proposed patch to the topic branch (which will be a little bit wrongly named, OTOH the only change it currently brings is precisely the .gitignore change), test it, adjust documentation and reassign to you for QA.

#21 Updated by intrigeri 2015-05-08 14:06:36

• Assignee changed from intrigeri to anonym
• QA Check changed from Dev Needed to Ready for QA

Applied the patch after splitting it into atomic commits.
Tested, works fine :)

#22 Updated by anonym 2015-05-11 05:38:26

• Status changed from In Progress to Fix committed
• % Done changed from 50 to 100

Applied in changeset commit:5a83baa6a030583227b78722769e6b0f2716e8fa.

#23 Updated by anonym 2015-05-11 05:40:18

• Assignee deleted (anonym)
• QA Check changed from Ready for QA to Pass

#24 Updated by BitingBird 2015-05-12 18:39:23

• Status changed from Fix committed to Resolved