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: support for bundle installation from private registries #120

Merged
merged 10 commits into from
Oct 21, 2022

Conversation

bdegeeter
Copy link
Contributor

@bdegeeter bdegeeter commented Sep 12, 2022

Closes #68

This fixes authentication for installation of bundles hosted in private registries

cnab-to-oci requires a dockerconfig for authentication to pull and inspect a bundle manifest before install.

This change gets the first .dockerconfigjson secret from the default service account or the service account specified via the AgentConfig's installationServiceAccount. The secret is mounted in the job as a volume via the AgentAction reconciler.

carolynvs and others added 7 commits September 7, 2022 10:55
* Update k8s to v0.24.1
* Update porter which has newer versions of containerd, cnab-go, and
  docker

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
* Bump porter to v1.0.0-beta.1

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>

* Bump deps for CVE-2022-1996

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>

* go mod tidy again for reasons

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
This updates our home page to remove the disclaimer about credential and parameter sets

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
…-plugins

Signed-off-by: Steven Gettys <s.gettys@f5.com>
Signed-off-by: Steven Gettys <s.gettys@f5.com>
Fixes getporter#68

Signed-off-by: Brian DeGeeter <76455484+bdegeeter@users.noreply.github.com>
@bdegeeter bdegeeter self-assigned this Sep 12, 2022
@getporterbot getporterbot added this to In Progress in Porter and Mixins Sep 12, 2022
@codecov
Copy link

codecov bot commented Sep 12, 2022

Codecov Report

Merging #120 (14be128) into main (71d9ff8) will decrease coverage by 0.12%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##             main     #120      +/-   ##
==========================================
- Coverage   78.07%   77.95%   -0.13%     
==========================================
  Files          12       12              
  Lines         976     1016      +40     
==========================================
+ Hits          762      792      +30     
- Misses        136      142       +6     
- Partials       78       82       +4     
Flag Coverage Δ
unit-tests 77.95% <75.00%> (-0.13%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
controllers/agentaction_controller.go 80.09% <75.00%> (-0.51%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@carolynvs
Copy link
Member

@bdegeeter Can you rebase this PR on the latest changes from master? I am seeing extra commits being rewritten from main in this PR (e.g. bumping deps).

Also I don't have a PR template yet for this repo yet but for any PR it is super helpful for the reviewers to have the following information:

  • The issue being fixed
  • A summary of the issue/bug/feature
  • A summary of the changes introduced in the pull request

@bdegeeter
Copy link
Contributor Author

I'll keep digging in a bit to hit the code coverage goals

@carolynvs
Copy link
Member

Don't stress out about the code coverage. It's just there as a nudge/reminder, but we aren't saying "coverage must never decrease" or anything in order to merge.

log.V(Log4Debug).Info("no image pull secret found for service account", "sa_namespace", instSvcAccount.Namespace, "sa_name", imgPullSec.Name, "secret_name", imgPullSec.Name)
}
log.V(Log4Debug).Info("found image pull secret found for service account", "sa_namespace", instSvcAccount.Namespace, "sa_name", imgPullSec.Name, "secret_name", imgPullSec.Name)
//TODO:(bdegeeter) grab the first dockerconfig json for now. Need to address multiple image pull secrets in a single service account
Copy link
Member

Choose a reason for hiding this comment

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

Can you make this into an issue in the repo so that it's easier to remember/track that more work is needed?

@carolynvs carolynvs self-assigned this Sep 15, 2022
@carolynvs
Copy link
Member

I won't get a chance to review until next week, we are trying to get a rc.2 out soon and that's taking up all my time.

Copy link
Member

@carolynvs carolynvs 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, thanks! 👍

@carolynvs
Copy link
Member

@bdegeeter Any idea why the test TestAPIs is failing?

@sgettys
Copy link
Collaborator

sgettys commented Oct 21, 2022

@bdegeeter Any idea why the test TestAPIs is failing?

@carolynvs I dug into it and it looks like there might just be a race condition in the test between when the waitForPorter function completes and when we actually validate the resource conditions. I know I've ran into some inconsistency with this in the past I'll look into adding something to that method to address the race condition better

@carolynvs carolynvs merged commit 1af4457 into getporter:main Oct 21, 2022
Porter and Mixins automation moved this from In Progress to Done Oct 21, 2022
@bdegeeter bdegeeter deleted the bdegeeter/issue68 branch October 24, 2022 17:10
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.

Add image pull secrets to AgentConfig
3 participants