Skip to content

Conversation

@csrwng
Copy link
Contributor

@csrwng csrwng commented Dec 15, 2015

Adds a new source type to builds (image) to pull content into the build directory.

@csrwng
Copy link
Contributor Author

csrwng commented Dec 15, 2015

@bparees @mfojtik ptal

Copy link
Contributor

Choose a reason for hiding this comment

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

@openshift/api-review for API change
@openshift/ui-review for UI impact

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx

Copy link
Member

Choose a reason for hiding this comment

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

@csrwng
Copy link
Contributor Author

csrwng commented Dec 15, 2015

@rhcarvalho

Copy link
Contributor

Choose a reason for hiding this comment

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

explicitly list the kinds that are supported.

@csrwng
Copy link
Contributor Author

csrwng commented Dec 17, 2015

addressed comments so far

@csrwng
Copy link
Contributor Author

csrwng commented Dec 17, 2015

[test]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to bb7ce11

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/7902/)

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this just "secret"? Do we need more than one kind of secret?

Copy link
Contributor

Choose a reason for hiding this comment

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

future proofing and make it obvious to the user what the secret is used for?

Copy link
Contributor

Choose a reason for hiding this comment

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

Future proofing when we add push secret? Don't buy it. Obvious to the
user? Maybe buy it.

On Thu, Dec 17, 2015 at 1:20 PM, Ben Parees [email protected]
wrote:

In pkg/build/api/v1/types.go
#6324 (comment):

@@ -171,6 +176,31 @@ type BuildSource struct {
SourceSecret *kapi.LocalObjectReference json:"sourceSecret,omitempty" description:"supported auth methods are: ssh-privatekey"
}

+// ImageSource describes an image that is used as source for the build
+type ImageSource struct {

  • // From is a reference to an ImageStreamTag, ImageStreamImage, or DockerImage to
  • // copy source from.
  • From kapi.ObjectReference json:"from" description:"reference to ImageStreamTag, ImageStreamImage, or DockerImage"
  • // Paths is a list of source and destination paths to copy from the image.
  • Paths []ImageSourcePath json:"paths" description:"paths to copy from image"
  • // PullSecret is a reference to a secret to be used to pull the image from a registry
  • // If the image is pulled from the OpenShift registry, this field does not need to be set.
  • PullSecret *kapi.LocalObjectReference json:"pullSecret,omitempty" description:"overrides the default pull secret for the source image"

future proofing and make it obvious to the user what the secret is used
for?


Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/pull/6324/files#r47940697.

Copy link
Contributor

Choose a reason for hiding this comment

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

not necessarily pushsecret, but i'm well past the point of assuming we can predict how this thing might have to evolve in the future.

but if you'll accept it solely on the "more obvious to a user" basis, i can live w/o convincing you we might want additional secrets in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

PullSecret is fine - secrets will probably get cleanup in v2 anyway.

On Thu, Dec 17, 2015 at 3:40 PM, Ben Parees [email protected]
wrote:

In pkg/build/api/v1/types.go
#6324 (comment):

@@ -171,6 +176,31 @@ type BuildSource struct {
SourceSecret *kapi.LocalObjectReference json:"sourceSecret,omitempty" description:"supported auth methods are: ssh-privatekey"
}

+// ImageSource describes an image that is used as source for the build
+type ImageSource struct {

  • // From is a reference to an ImageStreamTag, ImageStreamImage, or DockerImage to
  • // copy source from.
  • From kapi.ObjectReference json:"from" description:"reference to ImageStreamTag, ImageStreamImage, or DockerImage"
  • // Paths is a list of source and destination paths to copy from the image.
  • Paths []ImageSourcePath json:"paths" description:"paths to copy from image"
  • // PullSecret is a reference to a secret to be used to pull the image from a registry
  • // If the image is pulled from the OpenShift registry, this field does not need to be set.
  • PullSecret *kapi.LocalObjectReference json:"pullSecret,omitempty" description:"overrides the default pull secret for the source image"

not necessarily pushsecret, but i'm well past the point of assuming we can
predict how this thing might have to evolve in the future.

but if you'll accept it solely on the "more obvious to a user" basis, i
can live w/o convincing you we might want additional secrets in the future.


Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/pull/6324/files#r47958060.

@bparees
Copy link
Contributor

bparees commented Jan 4, 2016

lgtm. @openshift/api-review sign off?

@smarterclayton
Copy link
Contributor

API approved

@bparees
Copy link
Contributor

bparees commented Jan 4, 2016

[merge]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/4490/) (Image: devenv-rhel7_3068)

@bparees
Copy link
Contributor

bparees commented Jan 4, 2016

looks etcd flaky to me. [merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to bb7ce11

openshift-bot pushed a commit that referenced this pull request Jan 5, 2016
@openshift-bot openshift-bot merged commit e293eae into openshift:master Jan 5, 2016
@csrwng csrwng deleted the image_source branch January 27, 2016 14:43
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.

7 participants