Adding ConfigMap build sources to api #43
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openshift-ci-robot
added
the
size/XXL
|
ping @openshift/api-review |
liggitt
reviewed
Apr 25, 2018
build/v1/types.go
Outdated
| @@ -462,15 +466,32 @@ type ImageSourcePath struct { | |||
| DestinationDir string `json:"destinationDir" protobuf:"bytes,2,opt,name=destinationDir"` | |||
| } | |||
|
|
|||
| // LocalObjectBuildSource is a build source that is copied into a build from a Kubernetes | |||
| // key-value store, such as a `Secret` or `ConfigMap`. | |||
| type LocalObjectBuildSource interface { | |||
There was a problem hiding this comment.
we typically don't declare interfaces or methods on API types... if you want to adapt them like this, do so in application code, not in the API structs themselves
There was a problem hiding this comment.
I wanted to keep this interface within Origin proper, but most of the builder code acts upon API structs and not converted internal representations - see https://github.com/openshift/origin/blob/master/pkg/build/builder/docker.go as an example. Exposing this interface at the API level allows most the code for mounting Secrets and ConfigMaps to be unified, but may not be the best long-term path.
liggitt
reviewed
Apr 25, 2018
build/v1/types.go
Outdated
| // SecretBuildSource describes a secret and its destination directory that will be | ||
| // used only at the build time. The content of the secret referenced here will | ||
| // be copied into the destination directory instead of mounting. | ||
| type SecretBuildSource struct { | ||
| // secret is a reference to an existing secret that you want to use in your | ||
| // Secret is a reference to an existing secret that you want to use in your |
There was a problem hiding this comment.
revert this, comments follow the serialized names
liggitt
reviewed
Apr 25, 2018
build/v1/types.go
Outdated
| // build. | ||
| Secret corev1.LocalObjectReference `json:"secret" protobuf:"bytes,1,opt,name=secret"` | ||
|
|
||
| // destinationDir is the directory where the files from the secret should be | ||
| // DestinationDir is the directory where the files from the secret should be |
liggitt
reviewed
Apr 25, 2018
build/v1/types.go
Outdated
| // used only at the build time. The content of the configmap referenced here will | ||
| // be copied into the destination directory instead of mounting. | ||
| type ConfigMapBuildSource struct { | ||
| // ConfigMap is a reference to an existing configmap that you want to use in your |
There was a problem hiding this comment.
lowercase comments to match serialized names (used to generate API docs)
liggitt
reviewed
Apr 25, 2018
build/v1/types.go
Outdated
| @@ -618,6 +700,9 @@ type CustomBuildStrategy struct { | |||
|
|
|||
| // buildAPIVersion is the requested API version for the Build object serialized and passed to the custom builder | |||
| BuildAPIVersion string `json:"buildAPIVersion,omitempty" protobuf:"bytes,7,opt,name=buildAPIVersion"` | |||
|
|
|||
| // Configs is a list of additional configMaps that will be included in the custom build pod | |||
| Configs []ConfigSpec `json:"configs,omitempty" protobuf:"bytes,8,rep,name=configs"` | |||
There was a problem hiding this comment.
I had that debate myself - I'm OK with that convention.
liggitt
reviewed
Apr 25, 2018
build/v1/types.go
Outdated
|
|
||
| // configs represents a list of configmaps and their destinations that will | ||
| // be used for the build. | ||
| Configs []ConfigMapBuildSource `json:"configs,omitempty" protobuf:"bytes,9,rep,name=configs"` |
adambkaplan
force-pushed
the
feature/build-source-configs
branch
from
21d0274
to
861ca95
Compare
|
@liggitt feature is proposed for 3.11. API changes are stable, but shouldn't be merged until 3.10 is cut. |
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
adambkaplan
force-pushed
the
feature/build-source-configs
branch
from
861ca95
to
65c6054
Compare
openshift-ci-robot
added
size/XL
* New type `ConfigMapSource` in common `BuildSource` * Regenerated protobuf, swagger, and deepcopy. Trello card: https://trello.com/c/RMKJxJUm/1020-5-allow-using-a-configmap-as-an-input-to-a-build-builds
adambkaplan
force-pushed
the
feature/build-source-configs
branch
from
65c6054
to
fe60953
Compare
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
|
/lgtm /hold |
openshift-ci-robot
added
do-not-merge/hold
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
openshift-cherrypick-robot
pushed a commit
to openshift-cherrypick-robot/api
that referenced
this pull request
Feb 1, 2021
Drop status subresource from generated CRD manifests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ConfigMapBuildSourceCommonSpecTrello card: https://trello.com/c/RMKJxJUm/1020-5-allow-using-a-configmap-as-an-input-to-a-build-builds