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

Modified Decode() to use SplitN rather than Split #1126

Merged
merged 3 commits into from
Apr 20, 2022

Conversation

jessequinn
Copy link
Contributor

@jessequinn jessequinn commented Apr 11, 2022

Modified Decode() in docker config pkg to use SplitN rather than Split. This allows GCR auths to be split that contain multiple : characters.

Description

Related issues

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've added tests that prove my fix is effective or that my feature works.

@jessequinn jessequinn changed the title Modified Decode() to use SplitN rather than Split. This allows GCR au… Modified Decode() to use SplitN rather than Split Apr 11, 2022
@CLAassistant
Copy link

CLAassistant commented Apr 11, 2022

CLA assistant check
All committers have signed the CLA.

…ths to be split that contain multiple : characters.
@jessequinn jessequinn force-pushed the gcr_support branch 2 times, most recently from cae13f3 to 85a67f6 Compare April 12, 2022 12:51
@codecov
Copy link

codecov bot commented Apr 14, 2022

Codecov Report

Merging #1126 (f74a5a3) into main (1550730) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1126      +/-   ##
==========================================
- Coverage   57.96%   57.95%   -0.02%     
==========================================
  Files          71       71              
  Lines        9305     9305              
==========================================
- Hits         5394     5393       -1     
- Misses       3361     3362       +1     
  Partials      550      550              
Impacted Files Coverage Δ
pkg/docker/config.go 73.61% <100.00%> (ø)
pkg/operator/controller/ciskubebenchreport.go 50.99% <0.00%> (-1.60%) ⬇️
pkg/vulnerabilityreport/controller.go 54.57% <0.00%> (-1.22%) ⬇️
pkg/cmd/installer.go 43.00% <0.00%> (-0.35%) ⬇️
pkg/configauditreport/controller.go 62.46% <0.00%> (+0.92%) ⬆️
pkg/operator/controller/configauditreport.go 54.03% <0.00%> (+1.39%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1550730...f74a5a3. Read the comment docs.

@chen-keinan
Copy link
Contributor

chen-keinan commented Apr 18, 2022

@jessequinn were you able to build the image and load it to kind?

@jessequinn
Copy link
Contributor Author

jessequinn commented Apr 18, 2022

@jessequinn were you able to build the image and load it to kind?

yes. however, the image will not contain this change. building the image uses github directly from what i remember.

@chen-keinan
Copy link
Contributor

chen-keinan commented Apr 19, 2022

@jessequinn were you able to build the image and load it to kind?

yes. however, the image will not contain this change. building the image uses github directly from what i remember.

make docker-build will build it locally and create the this image:aquasec/starboard-operator:dev

Is this what you used?

@jessequinn
Copy link
Contributor Author

@jessequinn were you able to build the image and load it to kind?

yes. however, the image will not contain this change. building the image uses github directly from what i remember.

make docker-build will build it locally and create the this image:aquasec/starboard-operator:dev

Is this what you used?

yes. verbatim to the contribution info page.

@chen-keinan
Copy link
Contributor

chen-keinan commented Apr 19, 2022

I see, it should work. I'll check it myself now

@chen-keinan chen-keinan self-requested a review April 19, 2022 06:18
@jessequinn
Copy link
Contributor Author

considering the change is so small why would it not work?

@chen-keinan
Copy link
Contributor

chen-keinan commented Apr 19, 2022

considering the change is so small, why would it not work?

I agree it should be ok, will do a quick check.

@chen-keinan
Copy link
Contributor

chen-keinan commented Apr 19, 2022

LGTM!, thank you for the contribution, please rebase your branch with upstream

@chen-keinan chen-keinan merged commit b367c30 into aquasecurity:main Apr 20, 2022
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.

Support Service Account Keys defined in ImagePullSecrets
3 participants