-
Notifications
You must be signed in to change notification settings - Fork 158
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
Add SCM certificate management support #1218
Conversation
# Conflicts: # go.mod # go.sum # vendor/modules.txt
docs/resources/scm_certificate.md
Outdated
|
||
The following arguments are supported: | ||
|
||
* `region` - (Optional, String, ForceNew) The region in which to create the ELB certificate resource. |
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.
s/ELB certificate/SCM certificate
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.
done
docs/resources/scm_certificate.md
Outdated
|
||
```hcl | ||
# Load the certificate contents from the local files. | ||
resource "huaweicloud_scm_certificate" "certificate_2" { |
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.
Nits: better to use certificate_1 here and certificate_2 below :D
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.
fixed
} | ||
if HW_CERTIFICATE_PRIVATE_KEY_PATH == "" { | ||
t.Skip("HW_CERTIFICATE_PRIVATE_KEY_PATH must be set for BMS certificate tests") | ||
} |
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.
can be combined to one if, if HW_CERTIFICATE_PATH == "" || if HW...
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.
fixed
} | ||
} | ||
|
||
//return fmt.Errorf("Error creating HuaweiCloud SCM client") // |
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.
this line should be removed.
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.
removed
docs/resources/scm_certificate.md
Outdated
subcategory: "SSL Certificate Manager (SCM)" | ||
--- | ||
|
||
# huaweicloud\_scm\_certificate |
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.
\
is unnecessary
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.
has removed
docs/resources/scm_certificate.md
Outdated
## Example Usage | ||
|
||
```hcl | ||
# Load the certificate contents from the local files. |
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.
### Load the certificate contents from the local files
```hcl
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.
the sample description has been reorganized
docs/resources/scm_certificate.md
Outdated
private_key = file("/usr/local/data/certificate/cert_xxx/xxx_server.key") | ||
} | ||
|
||
# Write the contents of the certificate into the Terrafrom script. |
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.
ditto
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.
done
docs/resources/scm_certificate.md
Outdated
- `update` - Default is 10 minute. | ||
- `delete` - Default is 5 minute. | ||
|
||
## Error Codes |
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.
this section is unnecessary
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.
has removed
Name: "scm", | ||
Version: "v3", | ||
WithOutProjectID: true, | ||
}, |
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.
should add the testing in endpoints_test.go
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 was omitted and has been added now.
ForceNew: true, | ||
DiffSuppressFunc: utils.SuppressNewLineDiffs, | ||
}, | ||
"push_certificate": { |
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.
suggest name as:
target {
service = "xxx"
project = ["aaa", "bbb"]
}
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.
got it
|
||
func resourceScmCertificateV3Create(d *schema.ResourceData, meta interface{}) error { | ||
config := meta.(*config.Config) | ||
elbClient, err := config.ScmV3Client(GetRegion(d, config)) |
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.
the name of client is misleading
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.
has renamed
config := meta.(*config.Config) | ||
elbClient, err := config.ScmV3Client(GetRegion(d, config)) | ||
|
||
oldVal, newVal := d.GetChange("push_certificate") |
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.
suggest to define it as schema.Set, then we can use Difference
method to get the new target
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.
some logic has been updated.
|
||
rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(5)) | ||
targetService := "Enhance_ELB" | ||
defaultTargetProject := "ap-southeast-1" |
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.
hard coding is not recommend, please use HW_DEST_PROJECT
environment variable.
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 order to be consistent with other variables, use: HW_CERTIFICATE_TARGET_SERVICE
, HW_CERTIFICATE_TARGET_PROJECT
and HW_CERTIFICATE_TARGET_PROJECT_UPDATE
instead.
docs/resources/scm_certificate.md
Outdated
It can be extracted from the _server.crt_ file in the Nginx directory, | ||
usually after the second paragraph is the certificate chain. | ||
* `private_key` - (Required, String, ForceNew) The private encrypted key of the Certificate, PEM format. | ||
* `target` - (Optional, List) The service to which the certificate needs to be pushed.The push_certificate |
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.
still using push_certificate
in the description.
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.
removed
docs/resources/scm_certificate.md
Outdated
* `service` - (Required, String) Service to which the certificate is pushed. | ||
The options include `CDN`,`WAF` and `Enhance_ELB`. | ||
* `project` - (Optional, String) The project where the service you want to push a certificate to. | ||
The same certificate can be pushed repeatedly to the same WAF or ELB service in the same `target_project`, |
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.
still using target_project
in the description.
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.
fixed
log.Printf("[DEBUG] Find new services and start to push. %#v", pushOpts) | ||
err := doPushCertificateToService(d.Id(), pushOpts, scmClient) | ||
if err != nil { | ||
d.SetId("") |
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 not set the ID to null in Update method, just return the error.
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.
If id is not set to blank, the data will be saved to the state file. The change cannot be recognized when executed again.
To be safe, I overwrite the target
with the old value.
What this PR does / why we need it:
fixes #1179
Which issue this PR fixes:
NONE
Special notes for your reviewer:
Release note:
NONE
PR Checklist
Acceptance Steps Performed
make testacc TEST='./huaweicloud' TESTARGS='-run TestScmCertificationV3_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./huaweicloud -v -run TestScmCertificationV3_basic -timeout 360m -parallel 4
=== RUN TestScmCertificationV3_basic
=== PAUSE TestScmCertificationV3_basic
=== CONT TestScmCertificationV3_basic
--- PASS: TestScmCertificationV3_basic (18.96s)
PASS
ok github.com/huaweicloud/terraform-provider-huaweicloud/huaweicloud 19.075s
make testacc TEST='./huaweicloud' TESTARGS='-run TestScloud' TESTARGS='-run TestScmCertificationPushV3'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./huaweicloud -v -run TestScmCertificationPushV3 -timeout 360m -parallel 4
=== RUN TestScmCertificationPushV3_basic
=== PAUSE TestScmCertificationPushV3_basic
=== CONT TestScmCertificationPushV3_basic
--- PASS: TestScmCertificationPushV3_basic (8.38s)
PASS
ok github.com/huaweicloud/terraform-provider-huaweicloud/huaweicloud 8.472s
make testacc TEST='./huaweicloud' TESTARGS='-run TestAccScmCertificationV3_push'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./huaweicloud -v -run TestAccScmCertificationV3_push -timeout 360m -parallel 4
=== RUN TestAccScmCertificationV3_push
=== PAUSE TestAccScmCertificationV3_push
=== CONT TestAccScmCertificationV3_push
--- PASS: TestAccScmCertificationV3_push (22.18s)
PASS
ok github.com/huaweicloud/terraform-provider-huaweicloud/huaweicloud 22.250s
make testacc TEST='./huaweicloud' TESTARGS='-run TestAccScmCertificationV3_batchPush'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./huaweicloud -v -run TestAccScmCertificationV3_batchPush -timeout 360m -parallel 4
=== RUN TestAccScmCertificationV3_batchPush
=== PAUSE TestAccScmCertificationV3_batchPush
=== CONT TestAccScmCertificationV3_batchPush
--- PASS: TestAccScmCertificationV3_batchPush (11.35s)
PASS
ok github.com/huaweicloud/terraform-provider-huaweicloud/huaweicloud 11.442s