Support for external artifact config in pipeline config API. #4760
Conversation
varshavaradarajan
force-pushed
the
pipeline-config-api-external-artifacts
branch
from
0daa575 to
887aa8b
Compare
varshavaradarajan
changed the title
|
To test -
{
"label_template": "${COUNT}",
"lock_behavior": "lockOnFailure",
"name": "new_pipeline",
"materials": [
{
"type": "git",
"attributes": {
"url": "git@github.com:sample_repo/example.git",
"destination": "dest"
}
}
],
"stages": [
{
"name": "defaultStage",
"jobs": [
{
"name": "defaultJob",
"tasks": [
{
"type": "exec",
"attributes": {
"run_if": [
"passed"
],
"command": "ls",
"working_directory": null
}
}
],
"artifacts": [
{
"type": "build",
"source": "foo",
"destination": "bar"
},
{
"type": "test",
"source": "fml",
"destination": "fml"
},
{
"type": "external",
"artifact_id": "gross",
"store_id": "hub",
"configuration": [
{
"key": "Image",
"value": "we"
}
]
}
]
}
]
},
{
"name": "defaultStage2",
"jobs": [
{
"name": "defaultJob",
"tasks": [
{
"type": "exec",
"attributes": {
"run_if": [
"passed"
],
"command": "ls",
"working_directory": null
}
},
{
"type": "fetch",
"attributes": {
"origin": "gocd",
"stage": "defaultStage",
"job": "defaultJob",
"source": "dir"
}
},
{
"type": "fetch",
"attributes": {
"origin": "external",
"pipeline": "new_pipeline",
"stage": "defaultStage",
"job": "defaultJob",
"artifact_id": "gross",
"configuration": [
{
"key": "foo",
"value": "plain"
}
]
}
}
]
}
]
}
]
}
@akshaydewan - I guess the above applies to config repos as well. Although I don't think anything would have to change as the |
varshavaradarajan
force-pushed
the
pipeline-config-api-external-artifacts
branch
4 times, most recently
from
3273c16 to
5b753f2
Compare
varshavaradarajan
force-pushed
the
pipeline-config-api-external-artifacts
branch
2 times, most recently
from
5dfdc97 to
4eb2a6d
Compare
| break; | ||
| default: | ||
| //TODO: change this when support for external artifacts is introduced | ||
| throw new UnprocessableEntityException(String.format("Invalid Artifact type: '%s'. It has to be one of %s.", type, String.join(",", "build", "test"))); |
There was a problem hiding this comment.
add external to exception message
| this.getConfiguration().add(builder.create(property.getConfigKeyName(), | ||
| property.getConfigValue(), | ||
| property.getEncryptedValue(), | ||
| false)); |
There was a problem hiding this comment.
Always marking a configuration as unSecure can lead to ConfigurationPropertyBuilder adding errors if a encrypted value is provided.
| public void encryptSecureProperties(CruiseConfig cruiseConfig, PipelineConfig pipelineConfig) { | ||
| if (artifactId != null) { | ||
| PipelineConfig dependencyMaterial = null; | ||
| if (pipelineName == null || CaseInsensitiveString.isBlank(pipelineName.getPath())) { |
There was a problem hiding this comment.
We will have to handle a scenario where a given pipeline name corresponds to the current pipeline.
This could happen when a job fetches an artifact from a job corresponding to the previous stage in the pipeline.
| // ignore | ||
| } | ||
| } | ||
| if (dependencyMaterial != null) { |
| return; | ||
| } | ||
| Configuration configurationProperties = new Cloner().deepClone(getConfiguration()); | ||
| getConfiguration().clear(); |
There was a problem hiding this comment.
If the config is validated against the plugin and if there are errors it would be cleared as well.
| } | ||
|
|
||
| public void addConfigurations(List<ConfigurationProperty> configurationProperties, CruiseConfig cruiseConfig) { | ||
| //TODO: based on https://github.com/gocd/gocd/pull/4763 |
There was a problem hiding this comment.
Guess this needs to be removed.
varshavaradarajan
force-pushed
the
pipeline-config-api-external-artifacts
branch
2 times, most recently
from
a920975 to
17dd3f0
Compare
varshavaradarajan
force-pushed
the
pipeline-config-api-external-artifacts
branch
2 times, most recently
from
fc9a7f1 to
77d3e46
Compare
|
What we have finally decided on encryption:
|
|
Questions for @arvindsv: Context: In this PR, we handle encryption of secure publish and fetch plugin properties for pipelines. We have various config save flows that we should take care of:
For the various config save flows that we have, should we even preprocess and attempt to encrypt plugin properties at the template level given that at the template level given that the store id or artifact id may be a param and so we might not be able to encrypt the properties? Asking this so that we don't perform expensive param preprocessing (which won't be able to replace param values for anything in a template) if we can't encrypt. We could also try to encrypt the plugin properties as is without preprocessing, but we might end up running into #4863. For validation of a template publish or fetch external artifact config, the validation of plugin properties should happen in the context of the associated pipelines? So, for create template, we won't be requesting any plugin to perform validations, but for update of template, we have to check the associated pipelines and request the respective plugin to perform the validation, right? @jyotisingh, @maheshp - Have I missed anything? |
varshavaradarajan
force-pushed
the
pipeline-config-api-external-artifacts
branch
2 times, most recently
from
9cf18eb to
7fda3d5
Compare
varshavaradarajan
force-pushed
the
pipeline-config-api-external-artifacts
branch
from
5f4d8f9 to
d1a2cee
Compare
…all the pipelines or the relevant pipeline.
…s a param. Encrypt fetch external artifact task properties when external config identifiers are params.
* This is so that PipelineConfigErrorCopier can copy plugin properties error to the pipeline config object.
… to encrypt plugin properties.
* Fix tests
…. Artifact id could be a param, using NameTypeValidator will result in errors when a parameter is provided at the template level. * Currently for publish artifact, artifact id can't be a parameter. This needs to change at the xsd and validate level for templates.
varshavaradarajan
force-pushed
the
pipeline-config-api-external-artifacts
branch
from
9052bed to
f1f858e
Compare
3 participants
No description provided.