-
Notifications
You must be signed in to change notification settings - Fork 903
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(git/repo): add git/repo artifact support in kustomize bake manifest #7572
Conversation
@ezimanyi - tagging you so you're aware! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks fine, left a few suggestions and questions inline. Guessing we should hold this until the corresponding service changes are merged? Also just want to verify that this has been tested with the artifacts rewrite feature flag both enabled and disabled? The screenshot in the PR description shows an artifact inline in the bake stage, but also want to verify that one could define a git repo artifact in the Expected Artifacts section of the pipeline config. Thank you! 🙏
...s/modules/core/src/pipeline/config/stages/bakeManifest/kustomize/BakeKustomizeConfigForm.tsx
Outdated
Show resolved
Hide resolved
...s/modules/core/src/pipeline/config/stages/bakeManifest/kustomize/BakeKustomizeConfigForm.tsx
Outdated
Show resolved
Hide resolved
...cripts/modules/core/src/pipeline/config/triggers/artifacts/gitrepo/GitRepoArtifactEditor.tsx
Outdated
Show resolved
Hide resolved
<StageArtifactSelectorDelegate | ||
artifact={this.getInputArtifact().artifact} | ||
excludedArtifactTypePatterns={[]} | ||
excludedArtifactTypePatterns={excludeAllTypesExcept(ArtifactTypePatterns.GIT_REPO)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So now that we only accept the new git repo artifact type in Kustomize stages, will all previously configured Kustomize stages break?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the UI, yes. We expected this so we called Kustomize support Beta in the last release. We're leaving the old code in place (for now) so it should continue to work. We'll come up with a communication plan for when the old path will be removed.
|
||
return ( | ||
<> | ||
<StageConfigField label="URL" helpKey="pipeline.config.expectedArtifact.gitrepo.url"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a heads up that in new stages, we are trying to move to FormikStageConfig
/ FormikFormField
rather than StageConfigField
-- see ScriptStageConfig.tsx
for an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good to know! I'll check out that example, thanks. Is this something we want changed now or refactored after the merge?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can refactor after the merge, I think the rest of the Bake (Manifest) stage is still written with StageConfigField
so we'd probably want to swap them all out at once in a later commit so the styling is consistent.
…st/kustomize/BakeKustomizeConfigForm.tsx Co-Authored-By: Maggie Neterval <mneterval@google.com>
…st/kustomize/BakeKustomizeConfigForm.tsx Co-Authored-By: Maggie Neterval <mneterval@google.com>
…s/gitrepo/GitRepoArtifactEditor.tsx Co-Authored-By: Maggie Neterval <mneterval@google.com>
@KathrynLewis Awesome, thank you! 🙏 |
34a3b00 feat(managed): Update resource indicators to use new data source 7b33d2b feat(managed): Join managed data to infra data, add moniker to Security Groups d4c89ce feat(managed): Use new data source for 'Managed Resources' config section efcceb3 feat(managed): add 'managedResources' data source 4ed015a feat(dataSources): widen + parameterize types, add default values 8c29cba Reactify titus launch configuration (spinnaker#7581) 5d89a04 feat(google): add gce scale-down controls (spinnaker#7570) f882544 feat(git/repo): add git/repo artifact support in kustomize bake manifest (spinnaker#7572)
34a3b00 feat(managed): Update resource indicators to use new data source 7b33d2b feat(managed): Join managed data to infra data, add moniker to Security Groups d4c89ce feat(managed): Use new data source for 'Managed Resources' config section efcceb3 feat(managed): add 'managedResources' data source 4ed015a feat(dataSources): widen + parameterize types, add default values 8c29cba Reactify titus launch configuration (#7581) 5d89a04 feat(google): add gce scale-down controls (#7570) f882544 feat(git/repo): add git/repo artifact support in kustomize bake manifest (#7572)
…est (spinnaker#7572) * feat(git/repo): add git/repo artifact support in kustomize bake manifest * Update app/scripts/modules/core/src/pipeline/config/stages/bakeManifest/kustomize/BakeKustomizeConfigForm.tsx Co-Authored-By: Maggie Neterval <mneterval@google.com> * Update app/scripts/modules/core/src/pipeline/config/stages/bakeManifest/kustomize/BakeKustomizeConfigForm.tsx Co-Authored-By: Maggie Neterval <mneterval@google.com> * Update app/scripts/modules/core/src/pipeline/config/triggers/artifacts/gitrepo/GitRepoArtifactEditor.tsx Co-Authored-By: Maggie Neterval <mneterval@google.com>
34a3b00 feat(managed): Update resource indicators to use new data source 7b33d2b feat(managed): Join managed data to infra data, add moniker to Security Groups d4c89ce feat(managed): Use new data source for 'Managed Resources' config section efcceb3 feat(managed): add 'managedResources' data source 4ed015a feat(dataSources): widen + parameterize types, add default values 8c29cba Reactify titus launch configuration (spinnaker#7581) 5d89a04 feat(google): add gce scale-down controls (spinnaker#7570) f882544 feat(git/repo): add git/repo artifact support in kustomize bake manifest (spinnaker#7572)
…est (spinnaker#7572) * feat(git/repo): add git/repo artifact support in kustomize bake manifest * Update app/scripts/modules/core/src/pipeline/config/stages/bakeManifest/kustomize/BakeKustomizeConfigForm.tsx Co-Authored-By: Maggie Neterval <mneterval@google.com> * Update app/scripts/modules/core/src/pipeline/config/stages/bakeManifest/kustomize/BakeKustomizeConfigForm.tsx Co-Authored-By: Maggie Neterval <mneterval@google.com> * Update app/scripts/modules/core/src/pipeline/config/triggers/artifacts/gitrepo/GitRepoArtifactEditor.tsx Co-Authored-By: Maggie Neterval <mneterval@google.com>
34a3b00 feat(managed): Update resource indicators to use new data source 7b33d2b feat(managed): Join managed data to infra data, add moniker to Security Groups d4c89ce feat(managed): Use new data source for 'Managed Resources' config section efcceb3 feat(managed): add 'managedResources' data source 4ed015a feat(dataSources): widen + parameterize types, add default values 8c29cba Reactify titus launch configuration (spinnaker#7581) 5d89a04 feat(google): add gce scale-down controls (spinnaker#7570) f882544 feat(git/repo): add git/repo artifact support in kustomize bake manifest (spinnaker#7572)
…est (spinnaker#7572) * feat(git/repo): add git/repo artifact support in kustomize bake manifest * Update app/scripts/modules/core/src/pipeline/config/stages/bakeManifest/kustomize/BakeKustomizeConfigForm.tsx Co-Authored-By: Maggie Neterval <mneterval@google.com> * Update app/scripts/modules/core/src/pipeline/config/stages/bakeManifest/kustomize/BakeKustomizeConfigForm.tsx Co-Authored-By: Maggie Neterval <mneterval@google.com> * Update app/scripts/modules/core/src/pipeline/config/triggers/artifacts/gitrepo/GitRepoArtifactEditor.tsx Co-Authored-By: Maggie Neterval <mneterval@google.com>
34a3b00 feat(managed): Update resource indicators to use new data source 7b33d2b feat(managed): Join managed data to infra data, add moniker to Security Groups d4c89ce feat(managed): Use new data source for 'Managed Resources' config section efcceb3 feat(managed): add 'managedResources' data source 4ed015a feat(dataSources): widen + parameterize types, add default values 8c29cba Reactify titus launch configuration (spinnaker#7581) 5d89a04 feat(google): add gce scale-down controls (spinnaker#7570) f882544 feat(git/repo): add git/repo artifact support in kustomize bake manifest (spinnaker#7572)
This adds support for the git/repo artifact type when baking a manifest using kustomize. With this change, git/repo is the only supported artifact for kustomize.
Original PR adding git/repo artifact type: spinnaker/clouddriver#4049
Related PRs to this kustomize work: