-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Allow using in repo configuration for cloudbuild trigger #1557
Allow using in repo configuration for cloudbuild trigger #1557
Conversation
Cloudbuild triggers have a complex configuration that can be defined from the API. When using the console, the more typical way of doing this is to defined the configuration within the repository and point the configuration to the file that defines the config. This can be supported by sending the filename parameter instead of the build parameter, however only one can be sent.
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.
Looks great! Can you add a test?
Ensure that when a cloudbuild repo trigger is created with a filename, that filename is what actually ends up in the cloud.
I was certain of that response even when I submitted it :) I had to look up a bunch of how terraform testing works, and tried to follow the existing patterns. Let me know if there's something missing (or just fix it and push commits is fine with me). |
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.
Looks good - one docs tweak, then I'll run the tests and merge.
@@ -59,6 +74,10 @@ will be expanded when the build is created: | |||
or resolved from the specified branch or tag. | |||
* `$SHORT_SHA`: first 7 characters of `$REVISION_ID` or `$COMMIT_SHA`. | |||
|
|||
* `filename` - (Optional) Specify the path to a Cloud Build configuration file | |||
in the Git repo. This is mutually exclusive with `build`. By default this is |
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.
It doesn't seem like there's a default in here, so if you change 'by default' to 'Typically', that will be a bit clearer.
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, agreed. There's no default on the API, you have to specify a value. It's the web portal that suggests you use cloudbuild.yaml.
The docs shouldn't say that "cloudbuild.yaml" is used by default. There is no default from the APIs, but the console suggest using this value. Just say it's the typical value in documentation.
Can this be merged? |
Merged. Thanks Jamie for your contribution :) |
+1
…On Tue, Jun 5, 2018 at 1:18 PM Vincent Roseberry ***@***.***> wrote:
Merged. Thanks Jamie for your contribution :)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1557 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAFkGmGRQd5rOcNbO404tsvL8veMjcUoks5t5r1ogaJpZM4UQ-Ru>
.
|
…-google * 'master' of https://github.com/olvesh/terraform-provider-google: (24 commits) Cleanup after v1.14.0 release v1.14.0 Update CHANGELOG.md Add new google_compute_regions (hashicorp#1603) Update CHANGELOG.md Fix forwarding rule data source test (hashicorp#1606) Update CHANGELOG.md Fix redis authorized network and tests. The Redis API currently only accepts partial links. The tests weren't failing because they weren't actually using the network (oops). There were a few other test issues that I fixed while I was there. Fixes hashicorp#1571. (hashicorp#1599) update auth docs (hashicorp#1587) Fix network_tier tests. Add documentation for network tier (hashicorp#1593) Warn about ip_version with ip_address in global forwarding rule (hashicorp#616) Update CHANGELOG.md add support for network tiers (hashicorp#1530) Update CHANGELOG.md Allow using in repo configuration for cloudbuild trigger (hashicorp#1557) Update CHANGELOG.md add update support for redis (hashicorp#1590) Update CHANGELOG.md Added GCP Netblock Data Source (hashicorp#1416) (hashicorp#1580) ...
) * Allow using in repo configuration for cloudbuild trigger Cloudbuild triggers have a complex configuration that can be defined from the API. When using the console, the more typical way of doing this is to defined the configuration within the repository and point the configuration to the file that defines the config. This can be supported by sending the filename parameter instead of the build parameter, however only one can be sent. * Acceptance testing for cloudbuild trigger with filename Ensure that when a cloudbuild repo trigger is created with a filename, that filename is what actually ends up in the cloud. * Don't specify "by default" in cloudbuild-trigger. The docs shouldn't say that "cloudbuild.yaml" is used by default. There is no default from the APIs, but the console suggest using this value. Just say it's the typical value in documentation.
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
Cloudbuild triggers have a complex configuration that can be defined
from the API. When using the console, the more typical way of doing this
is to defined the configuration within the repository and point the
configuration to the file that defines the config.
This can be supported by sending the filename parameter instead of the
build parameter, however only one can be sent.