Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: #241 Anonymous authentication #408

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Feature: #241 Anonymous authentication #408

wants to merge 9 commits into from

Conversation

cezarypiatek
Copy link

@cezarypiatek cezarypiatek commented Dec 15, 2018

Pull Request (PR) description

This is an attempt of implementing the ability to configure credentials for anonymous authentication.
I added AnonymousCredentials parameter for xWebsite module. I've started adding UT but I've got stuck with this weird error related to CIM objects. Can anybody review my current changes and help me with this issue? Error message that I get from UT:

    [-] Error occurred in Context block 335ms
      ArgumentOutOfRangeException: Specified argument was out of the range of valid values.
      Parameter name: className
" to type "Microsoft.Management.Infrastructure.CimInstance". Error: "Specified argument was out of the range of valid values.
      Parameter name: className"
" to type "Microsoft.Management.Infrastructure.CimInstance". Error: "Specified argument was out of the range of valid values.
      Parameter name: className"
" to type "Microsoft.Management.Infrastructure.CimInstance". Error: "Specified argument was out of the range of valid values.als'. Cannot convert value "MSFT_xWebAnonymousAuthenticationCredentials
      Parameter name: className"
      at <ScriptBlock>, C:\Repositories\current\xWebAdministration\Tests\Unit\MSFT_xWebsite.Tests.ps1: line 557
      at DescribeImpl, C:\Program Files\WindowsPowerShell\Modules\Pester\4.3.1\Functions\Describe.ps1: line 161

This Pull Request (PR) fixes the following issues #241

Task list

  • Added an entry under the Unreleased section of the change log in the README.md.
    Entry should say what was changed, and how that affects users (if applicable).
  • Resource documentation added/updated in README.md.
  • Resource parameter descriptions added/updated in README.md, schema.mof
    and comment-based help.
  • Comment-based help added/updated.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Resource Testing Guidelines.
  • Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
  • New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

This change is Reviewable

@msftclas
Copy link

msftclas commented Dec 15, 2018

CLA assistant check
All CLA requirements met.

@stale
Copy link

stale bot commented Dec 29, 2018

Labeling this pull request (PR) as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on this PR is taken up again.

@stale stale bot added the abandoned The pull request has been abandoned. label Dec 29, 2018
@johlju
Copy link
Member

johlju commented Jan 26, 2019

@cezarypiatek Is the errors you getting unrelated to the changes in this PR? Could you please rebase against the dev branch. That will kick off the tests agai, and if the errors you are seeing is unrelated to the changes, maybe they pass this time.
It is also possible to kick off the tests again by closing and then directly reopening the same PR.

@cezarypiatek
Copy link
Author

cezarypiatek commented Jan 26, 2019

@johlju these errors are certainly related to my changes. I asked for help in #241 but nobody respond so far. There is something wrong with CIM object and I can't cope with this by myself. I need a help.

Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

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

I understand. Always happy to help, but I been overwhelmed at work last quarter, and guess @regedit32 also have been busy. Sorry it took so long for someone to look at this and help you.

I had a quick look at this and debugged it in VS Code. I saw a back tick that was placed wrong.

Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @cezarypiatek)

a discussion (no related file):
Could you please rebase against branch dev using git rebase (not by using git pull or git merge, to keep the commit history). If you don't know how to rebase your local dev and working branch, please look at how to Resolve merge conflicts.
Let me know if you need any assistance.



Tests/Unit/MSFT_xWebsite.Tests.ps1, line 560 at r1 (raw file):

-AnonymousCredentials  $MockAnonymousCredentials`

The back tick should be one space apart. This is what is giving the issue in the tests.
Also, there is an extra space between the parameter name and the variable name.

Should be: -AnonymousCredentials $MockAnonymousCredentials `

@stale stale bot removed the abandoned The pull request has been abandoned. label Jan 27, 2019
@codecov-io
Copy link

codecov-io commented Jan 27, 2019

Codecov Report

Merging #408 into dev will decrease coverage by 0.26%.
The diff coverage is 73.91%.

Impacted file tree graph

@@            Coverage Diff            @@
##              dev    #408      +/-   ##
=========================================
- Coverage   90.77%   90.5%   -0.27%     
=========================================
  Files          17      17              
  Lines        2438    2434       -4     
=========================================
- Hits         2213    2203      -10     
- Misses        225     231       +6
Impacted Files Coverage Δ
DSCResources/MSFT_xWebsite/MSFT_xWebsite.psm1 96.07% <73.91%> (-1.05%) ⬇️
...SCResources/MSFT_xIisLogging/MSFT_xIisLogging.psm1 97.58% <0%> (-0.17%) ⬇️
DSCResources/Helper.psm1 90% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e1778f...e665489. Read the comment docs.

@cezarypiatek
Copy link
Author

@johlju thanks for help, now I can continue my work with this feature.

@regedit32 regedit32 added the waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. label Jan 28, 2019
@stale
Copy link

stale bot commented Feb 11, 2019

Labeling this pull request (PR) as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on this PR is taken up again.

@stale stale bot added the abandoned The pull request has been abandoned. label Feb 11, 2019
@johlju
Copy link
Member

johlju commented Apr 16, 2019

@cezarypiatek Please see comments in the issue #241. Tag me here if that was not the cause of the problem and I can dig in further.
You can also debug the build worker by connecting to it by RDP if it helps. You could re-run the integration tests manually and test the code.

@johlju johlju changed the base branch from dev to master December 30, 2019 12:17
@johlju johlju changed the base branch from master to main January 7, 2021 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abandoned The pull request has been abandoned. waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Support for Editing Anonymous Authentication Credentials
5 participants