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

feat: switch to driver writing files #460

Merged
merged 1 commit into from May 12, 2021

Conversation

aramase
Copy link
Member

@aramase aramase commented Apr 5, 2021

Signed-off-by: Anish Ramasekar anish.ramasekar@gmail.com

Reason for Change:

  • Adds an optional feature gated with a flag --driver-write-secrets to allow the CSI driver to write secrets (this feature is available in CSI Driver v0.0.21+). Default is false. We will switch to true in the next releases based on testing.
  • Templates the e2e kind test pipeline
  • Removed the custom cloud env test pipeline as it wasn't testing anything. Instead added e2e tests for custom cloud environment with kind.
  • Adds test pipeline for with and without driver-write-secrets feature.

Requirements

  • squashed commits
  • included documentation
  • added unit tests and e2e tests (if applicable).

Issue Fixed:

fixes #475
fixes kubernetes-sigs/secrets-store-csi-driver#460

Does this change contain code from or inspired by another project?

  • Yes
  • No

If "Yes," did you notify that project's maintainers and provide attribution?

Special Notes for Reviewers:

@aramase aramase force-pushed the driver-write-file branch 2 times, most recently from a4caa6a to 3859e6e Compare April 5, 2021 22:35
@aramase aramase added this to the 0.0.15 milestone Apr 12, 2021
@aramase aramase force-pushed the driver-write-file branch 11 times, most recently from c450320 to f6cc67e Compare April 16, 2021 19:26
@aramase aramase marked this pull request as ready for review April 16, 2021 19:43
@aramase aramase force-pushed the driver-write-file branch 2 times, most recently from 269b350 to 294b454 Compare April 20, 2021 23:15
@codecov-commenter
Copy link

Codecov Report

Merging #460 (98db1ac) into master (5fed2b9) will decrease coverage by 1.28%.
The diff coverage is 38.23%.

❗ Current head 98db1ac differs from pull request most recent head 294b454. Consider uploading reports for the commit 294b454 to get more accurate results

@@            Coverage Diff             @@
##           master     #460      +/-   ##
==========================================
- Coverage   64.31%   63.02%   -1.29%     
==========================================
  Files           7        7              
  Lines         510      522      +12     
==========================================
+ Hits          328      329       +1     
- Misses        148      159      +11     
  Partials       34       34              

@aramase aramase requested review from ritazh and sozercan April 20, 2021 23:40
@aramase aramase requested a review from nilekhc May 10, 2021 17:39
@aramase aramase force-pushed the driver-write-file branch 2 times, most recently from 9baef5a to 671f447 Compare May 12, 2021 20:49
@aramase
Copy link
Member Author

aramase commented May 12, 2021

/azp run pr-e2e-azure

@azure-pipelines
Copy link

Azure Pipelines failed to run 1 pipeline(s).

@aramase
Copy link
Member Author

aramase commented May 12, 2021

/azp run pr-e2e-azure

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@aramase aramase requested a review from nilekhc May 12, 2021 22:32
Copy link
Contributor

@nilekhc nilekhc left a comment

Choose a reason for hiding this comment

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

lgtm

CI_KIND_CLUSTER: true
AZURE_ENVIRONMENT_FILEPATH: "/etc/kubernetes/custom_environment.json"
${{ if eq(driverWriteSecret, 'true') }}:
DRIVER_WRITE_SECRETS: true
Copy link
Member

Choose a reason for hiding this comment

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

nit: extra space here?

Copy link
Member Author

Choose a reason for hiding this comment

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

As it's part of the if clause the space is required

pkg/provider/provider.go Outdated Show resolved Hide resolved
Copy link
Member

@ritazh ritazh left a comment

Choose a reason for hiding this comment

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

LGTM
just few minor nits

Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>

ci: update pipeline to test feature

Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>

ci: update gotool to 1.16.3

Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>
@aramase aramase merged commit 40bb678 into Azure:master May 12, 2021
@aramase aramase deleted the driver-write-file branch May 12, 2021 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants