Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

KDC setup for Linux#6158

Merged
vijaykota merged 1 commit into
dotnet:dev/negotiatestreamfrom
rahulkotecha-zz:dev/negotiatestream
Feb 23, 2016
Merged

KDC setup for Linux#6158
vijaykota merged 1 commit into
dotnet:dev/negotiatestreamfrom
rahulkotecha-zz:dev/negotiatestream

Conversation

@rahulkotecha-zz
Copy link
Copy Markdown
Member

cc: @stephentoub, @bartonjs, @Priya91

  • Added KDC setup script and config files (tested on Ubuntu, Debian, CentOS and Red Hat)
  • Changes to System.Net.Security.Tests project file so as to copy the KDC setup script and related conf files to output directory

@dnfclas
Copy link
Copy Markdown

dnfclas commented Feb 17, 2016

Hi @rahulkotecha, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@dnfclas
Copy link
Copy Markdown

dnfclas commented Feb 17, 2016

@rahulkotecha, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
<ProjectGuid>{A55A2B9A-830F-4330-A0E7-02A9FB30ABD2}</ProjectGuid>
<OutputType>Library</OutputType>
<UnsupportedPlatforms>OSX</UnsupportedPlatforms>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: indentation

@bartonjs
Copy link
Copy Markdown
Member

cc @morganbr

exit 0
fi

configure_kdc
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If user already has krb5 installed this script will be a no-op. In such cases, the tests may fail (the user's krb5.conf may not contain the principals needed for the test)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to prompt the user to overwrite their set up if that's actually what they want?

What about an uninstall, that undoes all of the configuration done in the script?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prompting will not be possible due to the automated nature.

For uninstall, @stephentoub should we add an argument to the script? Then in #6139 should we also do an uninstall each time? (Note that always cleaning up will add to the time when devs run NegotiateStream tests locally)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prompting will not be possible due to the automated nature.

That's not necessarily true. There are many utilities that prompt, for example, unless you pass a /force or /Y or some such flag that says "I know what I'm doing, I automatically say 'yes' to any prompt, so don't prompt me".

For uninstall, @stephentoub should we add an argument to the script?

Yes, please.

Then in #6139 should we also do an uninstall each time?

If running the tests does an automatic install, then they should also automatically uninstall. Otherwise, running the tests leaves the box in a state different than it started. It should also be possible to run the uninstall manually and clean up any vestiges of the previous install.

Note that always cleaning up will add to the time when devs run NegotiateStream tests locally

We should make it so that it only uninstalls if it needed to install. That way, if a developer is working on this library and has manually run the install script, no extra install/uninstall time is taken in each run. If it's someone who's just running a full test pass or otherwise running the tests locally, and they don't have things set up appropriately, the tests will do the install and uninstall and their box will (hopefully) be left clean.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Thanks for the idea about force switch

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stephentoub we are not actually overwriting user's setup. If the kdc is already installed, we are assuming that the machine is already setup with required users for the testing (through earlier runs of the script). If this assumption is indeed correct, then we might not require the force switch.
Let me know if this assumption is correct.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rahulkotecha the force switch is also to let a dev running script outside CI to see some prompts before KDC config. Having the force switch allows the script to be run without prompts from test class setup

@stephentoub
Copy link
Copy Markdown
Member

Thanks, @rahulkotecha. A few more comments/questions, but overall LGTM.

@mmitche, could you take a peek as well?

@rahulkotecha-zz
Copy link
Copy Markdown
Member Author

incorporated all comments received so far and squashed.

@vijaykota
Copy link
Copy Markdown
Contributor

@mmitche please let us know if the changes look ok.

@vijaykota
Copy link
Copy Markdown
Contributor

@rahulkotecha there are some conflicts (probably because setup-kdc.sh has been checked in via Priya's commit)

1. KDC setup script and config files (tested on Ubuntu, Debian, CentOS and
Red Hat)
   - Provides option for installation and uninstallation of KDC
   - Prompts user for (un)installation and bypasses prompts when run with
	 'y' switch
   - Need to be run as sudoer/superuser

2. Changes to project file so as to copy the KDC setup script and related
conf files to output directory
@mmitche
Copy link
Copy Markdown
Member

mmitche commented Feb 22, 2016

Changes look good to me

@dotnet-bot test this please

@ellismg
Copy link
Copy Markdown
Contributor

ellismg commented Feb 22, 2016

Is there a reason we can't just stand up a Kerberos in Azure and depend on that, instead of requiring folks that want to run these tests do all the work to set things up?

vijaykota added a commit that referenced this pull request Feb 23, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.