knative / build-templates

A library of build templates.
Apache License 2.0
184 stars 68 forks source link

Optionally add pull secret for private ECR #82

Closed philippthun closed 5 years ago

philippthun commented 5 years ago

While testing with a private ECR, we figured out that the ecr_helper.sh script does not create the necessary pull secret of type kubernetes.io/dockerconfigjson. We added an optional flag --push-and-pull to the helper script to support private ECRs.

evankanderson commented 5 years ago

/ok-to-test

I don't really have an easy way to validate this, but it looks reasonable.

evankanderson commented 5 years ago

/lgtm /approve

AkihiroSuda commented 5 years ago

LGTM (IANAM)

In long-term, I expect Knative Build to support kubernetes.io/dockerconfigjson for pushing as well https://github.com/knative/build/issues/389

evankanderson commented 5 years ago

/assign @ImJasonH

Since I can't approve

philippthun commented 5 years ago

@ImJasonH - As this PR has been reviewed and approved by @evankanderson, would it be possible to merge it? Or is something missing from your point of view?

knative-prow-robot commented 5 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: evankanderson, ImJasonH, philippthun

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/knative/build-templates/blob/master/OWNERS)~~ [ImJasonH] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
philippthun commented 5 years ago

@evankanderson - I've fixed a linting error in the README.md (see diff: https://github.com/knative/build-templates/compare/6b7a3b26536caf40ed092554c98b3c1ffc7d05f3..90020a86d8187ce1cfa2885be99048ae19400d9b). Could you please have a look?

evankanderson commented 5 years ago

/lgtm

(Sorry, missed your comment)

philippthun commented 5 years ago

/retest