rhodecode-enterprise-ce Pull Request !2275

Closed

Pull request !2275 Created on by

csalgau

Issue #5536 - ability to disable server-side SSH key generation

Commit flow:

branch:default of u/csalgau/rhodecode-enterprise-ce-csalgau-drop → branch:default of rhodecode-enterprise-ce

Versions:

Pull request versions not available.

Collapse 3 commits

ver	Time	Author	Commit	Description
`latest`	Wed, 13 Feb 2019 23:16:57	csalgau	`r3208:7d47a18b6391`	Issue #5536 - another template typo
`v2`	Wed, 13 Feb 2019 21:51:03	csalgau	`r3207:55f6d5276de9`	Issue #5536 - template typo. also make variable names more explicit
`v1`	Wed, 13 Feb 2019 20:05:00	csalgau	`r3206:c02cc55651b4`	Issue #5536 - ability to disable server-side SSH key generation
3 commits hidden, click expand to show them.

Collapse all files

No files

rhodecode/tests/rhodecode.ini unresolved comments ¶ 0

	This file was removed from diff during updates to this pull-request. There are still outdated/unresolved comments attached to it.
					note marcink Wed, 13 Feb 2019 21:30:46 outdated `v1` `\|` Comment #21545 Edit Delete `\|` Just a proposal not a big deal to make it more explicit. Maybe ssh.enable_ui_key_generator ?

rhodecode/templates/admin/users/user_edit_ssh_keys_generate.mako unresolved comments ¶ 0

	This file was removed from diff during updates to this pull-request. There are still outdated/unresolved comments attached to it.
					note marcink Wed, 13 Feb 2019 21:32:13 outdated `v1` `\|` Comment #21546 Edit Delete `\|` I wonder if we should put here a link for users with example key generation ? `ssh-keygen -t rsa -b 4096 -C "your_email@example.com"` note csalgau Wed, 13 Feb 2019 22:21:39 author outdated `v1` `\|` Comment #21550 Edit Delete `\|` In theory, users shouldn't be able to see this string without pasting or manually adding /generate in their URL. I think having the endpoint fail nicely is better than 404-ing. Unless you're going for easter eggs on hidden URL, I would rather suggest adding instructions to the main key management page. note marcink Wed, 13 Feb 2019 22:24:15 outdated `v1` `\|` Comment #21551 Edit Delete `\|` Right, this is the generate page, missed that. Yes this makes sense. todo marcink Wed, 13 Feb 2019 23:24:45 outdated `v2` `\|` Comment #21564 Edit Delete `\|` Actually Jenkins is right :D elif needs a condition too. I believe this should be %else fix #21564 csalgau Thu, 14 Feb 2019 00:18:09 author Comment #21566 Edit Delete `\|` I did have that fixed on the live template. I just didn't sync it.

rhodecode/templates/admin/my_account/my_account_ssh_keys.mako unresolved comments ¶ 0

	This file was removed from diff during updates to this pull-request. There are still outdated/unresolved comments attached to it.
					note marcink Wed, 13 Feb 2019 21:33:13 `v1` `\|` Comment #21547 Edit Delete `\|` +1 for spelling fix ;) todo marcink Wed, 13 Feb 2019 22:28:02 outdated `v1` `\|` Comment #21554 Edit Delete `\|` While jenkins tests this and shows the error, i'll point out that this `endif` generates syntax error. Should be `% endif` note csalgau Wed, 13 Feb 2019 22:30:37 author outdated `v1` `\|` Comment #21555 Edit Delete `\|` indeed I goofed on that. I'm working with a convoluted docker on windows setup and diffing things manually. fix #21554 csalgau Wed, 13 Feb 2019 22:52:29 author `v2` `\|` Comment #21558 Edit Delete `\|` fixed in r3207

General Comments 23

csalgau

v1 |

|

Auto status change to "Under Review"

note

jenkins-tests

v1 |

|

Build Failed: http://internal-jenkins.rhodecode.com/job/rhodecode-enterprise-ce-public-prs/9//console

note

csalgau

v1 |

|

Changed one (untranslated) string, added one new string.
I'm writing against a running test instance. Did not touch translations folder.

note

marcink

v1 |

|

Let me change the CI config so it sends the test results in a comment.

note

csalgau

v1 |

|

I meant to write

Changed two (untranslated) strings, added one new string.

jenkins-tests

v1 |

|

[PR tests] Build Failed on : build-log

cla-bot

v1 |

|

CLA FOUND and APPROVED

note

csalgau

v1 |

|

I'm not clear on how the build log from the CI jenkins bot should work. I'm guessing you're still working on that.

note

marcink

v1 |

|

There's a failing test, let me fix Jenkins to show the output.

jenkins-tests

v1 |

|

[PR tests] Build Failed on : build-log

csalgau

v2 |

|

Pull request updated. Auto status change to "Under Review"

Changed commits:
  * 1 added
  * 0 removed

Changed files:
  * M configs/development.ini
  * M configs/production.ini
  * M docs/auth/ssh-connection.rst
  * M rhodecode/apps/my_account/views/my_account_ssh_keys.py
  * M rhodecode/apps/ssh_support/__init__.py
  * M rhodecode/apps/ssh_support/config_keys.py
  * M rhodecode/lib/base.py
  * M rhodecode/templates/admin/my_account/my_account_ssh_keys.mako
  * M rhodecode/templates/admin/users/user_edit_ssh_keys.mako
  * M rhodecode/templates/admin/users/user_edit_ssh_keys_generate.mako
  * M rhodecode/tests/rhodecode.ini

cla-bot

v2 |

|

CLA FOUND and APPROVED

note

csalgau

v2 |

|

This is entirely text editor based right now. I'll be in a much better position to test things with #5534 in place :)

jenkins-tests

v2 |

|

[PR tests] Build Failed: build-log

note

csalgau

v2 |

|

@marcink pretty sure@jenkins-tests posted the wrong build log. it's the same log as last time.

note

marcink

v2 |

|

Yes, sorry about that. We did lots of Jenkins Changes receantly, and seems there are hickups!

Let me investigate.

csalgau

|

Pull request updated. Auto status change to "Under Review"

Changed commits:
  * 1 added
  * 0 removed

Changed files:
  * M rhodecode/templates/admin/users/user_edit_ssh_keys_generate.mako

cla-bot

|

CLA FOUND and APPROVED

note

csalgau

|

So I'm going to make a small complaint about the fact that the public repository is missing the 4.15 branch and my test instance is up-to-date. This makes syncing things less than straight forward. Also, for some reason it looked to me like both build bot posts pointed to the same file. Looks ok now.

note

marcink

|

Yeah, i'd realized that now that infact 4.15 is not pushed to the public instance... This should be done automatically.
I'll make sure we re-sync the codebase today.

jenkins-tests

|

Build Succeeded!

marcink

|

Thank you for this contribution. This looks ok, i'll merge this manually on top of our current default develop branch.

marcink

|

Closing with status change > Approved.

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages