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

support for providing multiple CMs and secrets in fn create #1282

Merged
merged 7 commits into from Aug 30, 2019

Conversation

@viveksinghggits
Copy link
Contributor

commented Aug 20, 2019

This change fixes the issue #1055 .


This change is Reviewable

@life1347

This comment has been minimized.

Copy link
Member

commented Aug 21, 2019

@viveksinghggits Thanks for the PR. Would you mind adding test to test/tests/test_secret_cfgmap to test against with the changes?

@viveksinghggits

This comment has been minimized.

Copy link
Contributor Author

commented Aug 21, 2019

@viveksinghggits Thanks for the PR. Would you mind adding test to test/tests/test_secret_cfgmap to test against with the changes?

@life1347 I will add the tests and update.

@viveksinghggits

This comment has been minimized.

Copy link
Contributor Author

commented Aug 23, 2019

hi @life1347
to write the test cases I was thinking of creating another template .py file that will read the two provided configmaps/secrets and then have a separate function (line checkFunctionResponse) that will call above function to check if two configmaps/secrets are being returned or not.
My concern was if I should created another function like checkFunctionResponse or not.
cc @vishal-biyani

@life1347

This comment has been minimized.

Copy link
Member

commented Aug 27, 2019

How about writing a fission function that combines multiple value of confimaps/secret into one line response and then use checkFunctionResponse to check the response body. In this way, you don't need to write additional bash function.

UPDATE:
You don't need to write another function like checkFunctionResponse, you can directly use it to check the value as follows

# from test_secret_cfgmap.sh
timeout 60 bash -c "checkFunctionResponse ${fn_secret} 'TESTVALUE' 'secret'"
@viveksinghggits

This comment has been minimized.

Copy link
Contributor Author

commented Aug 27, 2019

Hi @life1347
that sounds good to me, I will make the changes and update.
Update:
Added the tests.

@viveksinghggits

This comment has been minimized.

Copy link
Contributor Author

commented Aug 30, 2019

@life1347
can we get this merged.

@life1347

This comment has been minimized.

Copy link
Member

commented Aug 30, 2019

@viveksinghggits yes, I will get this merged today.

@life1347

This comment has been minimized.

Copy link
Member

commented Aug 30, 2019

LGTM & CI integration build passed: https://travis-ci.org/fission/fission/builds/578692761

@life1347 life1347 merged commit cd36047 into fission:master Aug 30, 2019

6 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - environments/jvm/pom.xml (soamvasani) No new issues
Details
security/snyk - environments/nodejs/package.json (soamvasani) No manifest changes detected
security/snyk - environments/python/requirements.txt (soamvasani) No manifest changes detected
security/snyk - environments/ruby/Gemfile.lock (soamvasani) No manifest changes detected
security/snyk - examples/nodejs/package.json (soamvasani) No manifest changes detected
@life1347

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

@viveksinghggits would you mind updating fission doc for multiple CMs & secrets access?

@viveksinghggits

This comment has been minimized.

Copy link
Contributor Author

commented Sep 6, 2019

Hi @life1347
I should have done that, forgot I think. Will do that and update.

@viveksinghggits

This comment has been minimized.

Copy link
Contributor Author

commented Sep 6, 2019

Hi @life1347
I have update the document and raised PR
fission/docs.fission.io#161

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.