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: allow empty objects array #671

Merged
merged 2 commits into from Dec 2, 2021

Conversation

onlined
Copy link
Contributor

@onlined onlined commented Oct 8, 2021

Reason for Change:
The YAML for secret provider class can be created by a template engine and all the objects listed there can depend on a template variable to be truthy. In this case, when all of them are falsy, it becomes a special case if empty objects array is not allowed. Example:

objects: |
  array:
{{ if condition1 }}
   - |
     objectName: "Secret1"
     objectType: secret
     objectFormat: pfx
     objectEncoding: base64
{{ end }}
{{ if condition2 }}
   - |
     objectName: "Secret2"
     objectType: secret
     objectFormat: pfx
     objectEncoding: base64
{{ end }}

If condition1 and condition2 are both falsy, the provider returns an error, but simply mounting an empty volume would be more consistent and it doesn't require special handling.

Requirements

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

Issue Fixed:

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

  • Yes
  • No

Special Notes for Reviewers:

@codecov-commenter
Copy link

codecov-commenter commented Oct 8, 2021

Codecov Report

Merging #671 (6704e81) into master (b35ae5f) will decrease coverage by 0.65%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #671      +/-   ##
==========================================
- Coverage   62.74%   62.09%   -0.66%     
==========================================
  Files           7        7              
  Lines         765      765              
==========================================
- Hits          480      475       -5     
- Misses        252      257       +5     
  Partials       33       33              

@onlined
Copy link
Contributor Author

onlined commented Oct 8, 2021

/azp run pr-e2e-azure

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 671 in repo Azure/secrets-store-csi-driver-provider-azure

@aramase
Copy link
Member

aramase commented Oct 8, 2021

/azp run pr-e2e

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@aramase aramase changed the title Allow empty objects array feat: allow empty objects array Oct 25, 2021
@github-actions
Copy link

github-actions bot commented Nov 9, 2021

This PR is stale because it has been open 14 days with no activity. Please comment or this will be closed in 7 days.

@github-actions github-actions bot added the stale label Nov 9, 2021
@aramase aramase removed the stale label Nov 9, 2021
@onlined
Copy link
Contributor Author

onlined commented Nov 9, 2021

@aramase Can you review this? It's OK if we don't merge for some time.

@github-actions
Copy link

This PR is stale because it has been open 14 days with no activity. Please comment or this will be closed in 7 days.

@github-actions github-actions bot added the stale label Nov 24, 2021
@onlined
Copy link
Contributor Author

onlined commented Nov 24, 2021

@aramase ?

@aramase
Copy link
Member

aramase commented Dec 2, 2021

/azp run pr-e2e

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@aramase
Copy link
Member

aramase commented Dec 2, 2021

/azp run pr-e2e-azure

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

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

Copy link
Member

@aramase aramase left a comment

Choose a reason for hiding this comment

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

lgtm

@aramase
Copy link
Member

aramase commented Dec 2, 2021

/azp run pr-e2e

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@aramase aramase enabled auto-merge (squash) December 2, 2021 21:21
@aramase aramase merged commit 26c6319 into Azure:master Dec 2, 2021
@onlined onlined deleted the allow-empty-objects-array branch December 2, 2021 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants