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

Export API keys for search service #3065

Merged
merged 9 commits into from
Jun 16, 2023
Merged

Conversation

super-harsh
Copy link
Collaborator

@super-harsh super-harsh commented Jun 12, 2023

What this PR does / why we need it:

Closes: #3053

Adding the support for exporting API keys for Azure Cognitive Search and reviving the authOptions property to enable users to select aadOrApiKey auth mode.

Special notes for your reviewer:

Also had to add the custom key matcher logic to replace/hide the base64 keys without '=' padding for the returned API keys

If applicable:

  • this PR contains tests

Copy link
Member

@theunrepentantgeek theunrepentantgeek left a comment

Choose a reason for hiding this comment

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

Changes look good for exposing the keys, but how are you addressing the ARM API shape required for apiKeyOnly access?

The required ARM payload is this:

"authOptions": {
    "apiKeyOnly": {}
}

I don't see any changes to the ARM types or the CRD shape in this PR to address this.

@super-harsh super-harsh self-assigned this Jun 12, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jun 12, 2023

Codecov Report

Merging #3065 (0f8a8aa) into main (eb24f83) will increase coverage by 0.00%.
The diff coverage is 60.00%.

@@           Coverage Diff            @@
##             main    #3065    +/-   ##
========================================
  Coverage   54.26%   54.26%            
========================================
  Files        1399     1400     +1     
  Lines      599240   599548   +308     
========================================
+ Hits       325179   325372   +193     
- Misses     220735   220815    +80     
- Partials    53326    53361    +35     
Impacted Files Coverage Δ
...v1api20220901/search_service_spec_arm_types_gen.go 25.00% <ø> (ø)
...h/v1api20220901storage/search_service_types_gen.go 55.17% <ø> (ø)
v2/internal/testcommon/test_context.go 62.68% <40.00%> (-0.58%) ⬇️
...i/search/v1api20220901/search_service_types_gen.go 58.66% <57.56%> (+1.13%) ⬆️
.../search/customizations/search_service_extension.go 70.14% <70.14%> (ø)

... and 33 files with indirect coverage changes

Copy link
Member

@theunrepentantgeek theunrepentantgeek left a comment

Choose a reason for hiding this comment

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

Looks good.

name: searchservicekeyssecret
queryKey:
key: queryKey
name: searchservicekeyssecret
Copy link
Member

Choose a reason for hiding this comment

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

minor: should have a newline at the end here

@super-harsh super-harsh merged commit c6211a9 into main Jun 16, 2023
@super-harsh super-harsh deleted the feature/search-extension branch June 16, 2023 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Improvement: Azure Cognitive Search: Auth mode and Keys
4 participants