-
Notifications
You must be signed in to change notification settings - Fork 57
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
SUMO-227092: Terraform support for azure source #626
Conversation
09cf2e3
to
dea6bcc
Compare
To retry with fixed environment variables. This reverts commit b02130e.
…form-fix-env Sumo 227092 azure source terraform fix env
Thanks Vijit for bringing up the Sensitive field here regarding the issue with password field. I tried that one but unfortunately even with Sensitive to true, it doesn't function properly and we will still have the same issue. Existing fields like secret_key in S3 and private_key in GCP metrics are all using plain text, so I think this behavior is acceptable for share_access_policy_key as well. We had discussions about cutoffTimestamp in slack and weekly sync. To summarize, with current changes, user can only update cutoffTimestamp to a non-zero value with tf, this is a discrepancy in tf that we have to live with. If users do want to update cutoffTimestamp to 0, they can do it via API, which we believe is a very rare case and to achieve "Collection from Now", user can alternatively just give the exact epoch timestamp of Now. And we have confirmed ingestion won't be affected. |
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 you check with Damo in #collection-rwc-dev-qe to see if QE needs to do testing on new TF source? If needed can they squeeze this in for S2?
Can I get one more approval for this PR? Thanks! cc @samjsong @vsinghal13 @yuting-liu |
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.
LGTM overall. One small question
Thanks Yuting for reviewing. Even though we make the scan_interval optional, I am inclined to not exposing this change to customer. The reason is that this change is only for us to handle scan_interval properly, customers don't need to do any changes for azure sources or their existing sources (s3 etc). Besides, I feel it's good practice for the customer to specify scan_interval explicitly. Wdyt? |
Addressed comments in #590. Closing the last pr since the base branch is too old.
I was able to create the source with the dev build and acceptance tests can pass now.