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.

Sign up for GitHub

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

Jump to bottom

Add Audit Logging To SecreteManager.exe #4183

Merged
merged 88 commits into from
Nov 26, 2024
Merged

Add Audit Logging To SecreteManager.exe #4183

merged 88 commits into from
Nov 26, 2024

Conversation

davfost
Copy link
Contributor

@davfost davfost commented Sep 27, 2024 •
edited
Loading

Add Audit Logging To SecreteManager.exe

  • Adds the OpenTelemetry.Audit.Geneva package and dependencies
  • Following logging recommendations from Geneva team
  • Added non-volatile logging operations to secret update and create operations
  • Adds new command line options for setting the target service ID to used when performing audit logging

Part Of GitHub Issue https://github.com/dotnet/arcade/issues/13217

@davfost davfost self-assigned this Sep 27, 2024
garath
garath reviewed Oct 7, 2024
View reviewed changes
NuGet.config Outdated Show resolved Hide resolved
@davfost
Updates to remove IF dependency for OpenTelemetry.Audit.Geneva in Azu…
720c50b 
…reKeyVault.cs.
@davfost
Removing packageSourceMapping and reference to AzureGenevaMonitoring …
5753848 
…feed to try to address the build failures seen in the PR build.
@davfost
Attempting to add explicit packageSourceMapping for Microsoft.VisualS…
0450b77 
…tudio.Azure.Fabric.MSBuild to address PR build breaks.
@davfost
Adding generic packageSourceMapping entires to try to address the PR …
714992f 
…build failures.
@davfost
Adding conditional setting for OpenTelemetry.Audit.Geneva refernce in…
5115f0c 
… Directory.Packages.props
@davfost
Moving the AzureGenevaMonitoring source mapping to the bottom of the …
522f80f 
…mapping list to see if that will ensure it is tested last.
@davfost
Removing the AzureGenevaMonitoring mapping instruction to see if it c…
9463454 
…lear the PR build issue.
@davfost
Removing the source reference for AzureGenevaMonitoring to verify tha…
a699252 
…t clears the PR build issue.
@davfost
Attempting to specify a custom nuget.config file from Directory.Build…
8964ec6 
….props so the down level Build Publish task can use the original nuget.config file for publish actions.
@davfost
Adding a conditional setting for the nuget.confg file path override i…
a9613e6 
…n Directory.Build.props
@davfost
Attempting to dynamically add the geneva audit log nuget feed using a…
c698a11 
… RestoreAdditionalProjectSources setting in the Directory.Build.props file instead of using a conditional nuget file path.
@davfost
Removing all conditional setting for nuget feeds specified in props a…
2eb417a 
…nd prog files.

The new plan will be to specified a cusotm nuget.config file named nuget.internal.config for interal build as a command line argument.

Example:

dotnet build ./dnceng.sln -p:DefineConstants="INTERNAL" --configfile "nuget.internal.config"
@davfost
Updating comment after latest PR changes.
2134f20 
Also removing an extra blank line in the Directory.Build.props file
davfost and others added 2 commits November 25, 2024 09:54
@davfost @garath
Update src/SecretManager/Microsoft.DncEng.SecretManager/Program.cs
482ceb9 
Co-authored-by: Michael Stuckey <[email protected]>
@davfost
Updates for comments in PR
ed2a44f 
Removing AzDo DevOps specific warning message formatting
Removing guidance messages being stored in ID values written to Audit Logs when IDs are not found.
garath
garath previously approved these changes Nov 26, 2024
View reviewed changes
Copy link
Member

@garath garath left a comment

Choose a reason for hiding this comment

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

All my questions for this iteration are answered!

@davfost
Removing Service Tree Id's empty Guid warning from Secret Manager Com…
6f58f24 
…mand objects.

Removing guidance string placed in ID values when claims properties are not found during identity data extraction.
@davfost davfost merged commit 60d8e85 into main Nov 26, 2024
7 checks passed
@davfost davfost deleted the dev/davfost/20240710 branch November 26, 2024 21:21
This was referenced Nov 26, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmitche mmitche mmitche left review comments

@dougbu dougbu dougbu left review comments

@garath garath garath approved these changes

@riarenas riarenas Awaiting requested review from riarenas

@epananth epananth Awaiting requested review from epananth

@missymessa missymessa Awaiting requested review from missymessa

Assignees

@davfost davfost

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@davfost @garath @dougbu @mmitche