Skip to content
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

terraform: remove cloud loggers #2892

Merged
merged 15 commits into from
Feb 6, 2024
Merged

Conversation

msanft
Copy link
Contributor

@msanft msanft commented Feb 5, 2024

Context

With the deprecation of Azure's Application Insights, we need to migrate away from using them for cloud logs. After a discussion, we concluded to remove cloud logging altogether, as we aggregate such logs in our CI through OpenSearch, and customers can use CSP APIs or deploy their own logging infrastructure to get to the boot logs.

Proposed change(s)

  • Remove cloud logging in the bootstrapper.
  • Remove creation of cloud logging resources through Terraform. For GCP, this is a project-wide API enablement, which we don't manage through Terraform, so the PR does not include a change for GCP here.
  • Remove the logging endpoint in the QEMU metadata API.
  • Overhaul the docs regarding getting to boot logs of Constellation nodes.

Important

When Azure disallows the creation of new application insights resources, users won't be able to create new clusters with an old Constellation version still relying on said resource. Thus, this change should be included in a release before the resource is abandoned. Existing clusters should not break, given there is a brownout period for existing application insights resources.

Additional info

Checklist

  • Run the E2E tests that are relevant to this PR's changes
  • Update docs
  • Add labels (e.g., for changelog category)
  • Is PR title adequate for changelog?
  • Link to Milestone

@msanft msanft added the breaking change Change breaks existing API or configuration. label Feb 5, 2024
@msanft msanft added this to the v2.16.0 milestone Feb 5, 2024
Copy link

netlify bot commented Feb 5, 2024

Deploy Preview for constellation-docs canceled.

Name Link
🔨 Latest commit f21d9d2
🔍 Latest deploy log https://app.netlify.com/sites/constellation-docs/deploys/65c20af7400e1f0008c8144d

Copy link
Contributor

@malt3 malt3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

go.mod Outdated Show resolved Hide resolved
@msanft msanft force-pushed the feat/terraform/remove-cloud-loggers branch from 52e7349 to c5be663 Compare February 5, 2024 13:00
msanft and others added 4 commits February 5, 2024 14:04
Co-authored-by: Daniel Weiße <66256922+daniel-weisse@users.noreply.github.com>
docs/docs/workflows/troubleshooting.md Outdated Show resolved Hide resolved
docs/docs/workflows/troubleshooting.md Outdated Show resolved Hide resolved
docs/docs/workflows/troubleshooting.md Outdated Show resolved Hide resolved
msanft and others added 3 commits February 6, 2024 09:47
Co-authored-by: Thomas Tendyck <51411342+thomasten@users.noreply.github.com>
Co-authored-by: Thomas Tendyck <51411342+thomasten@users.noreply.github.com>
Co-authored-by: Thomas Tendyck <51411342+thomasten@users.noreply.github.com>

You can view this information in the following places:
To debug issues occurring at boot time of the nodes, you can use the serial console interface of the CSP.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this option is actually disabled everywhere where its possible for non debug deployments.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also seeing the output? Or only logging in?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verifying this currently. I think we should have a read-only console on all CSPs

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you mean the serial console is disabled via the CSP, but not in a CC-secure way, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the read-only configuration is embedded in our image and thus also "CC-secure", but the cloud provider may preserve the logs after boot, which we seemingly explicitly turn off where possible. However, the serial console in all cases is read-only and, using our image, doesn't disclose any sensible information, so I don't think it's a problem.

Copy link
Member

@thomasten thomasten Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I didn't mean that there's a security issue. I only remembered that, in addition, we may have turned off serial console access/preserving via VM settings as a kind of best-effort approach.

Copy link
Contributor

github-actions bot commented Feb 6, 2024

Coverage report

Package Old New Trend
bootstrapper/cmd/bootstrapper [no test files] [no test files] 🚧
hack/qemu-metadata-api/server 60.70% 57.90% ↘️
internal/cloud/aws 84.40% 84.50% ↗️
internal/cloud/azure 67.10% 70.00% ↗️
internal/cloud/gcp 72.90% 75.90% ↗️
internal/cloud/qemu [no test files] [no test files] 🚧

@msanft msanft merged commit 901edd4 into main Feb 6, 2024
13 checks passed
@msanft msanft deleted the feat/terraform/remove-cloud-loggers branch February 6, 2024 13:27
msanft added a commit that referenced this pull request Feb 15, 2024
* terraform: remove cloud logging apps

* internal/cloud: remove loggers

* bootstrapper: remove logging

* qemu-metadata-api: remove logging endpoint

* docs: add instructions on how to get boot logs

* bazel: tidy

* docs: fix typo

* cloud: remove unused types

* Update go.mod

Co-authored-by: Daniel Weiße <66256922+daniel-weisse@users.noreply.github.com>

* bazel: tidy

* Update docs/docs/workflows/troubleshooting.md

Co-authored-by: Thomas Tendyck <51411342+thomasten@users.noreply.github.com>

* Update docs/docs/workflows/troubleshooting.md

Co-authored-by: Thomas Tendyck <51411342+thomasten@users.noreply.github.com>

* Update docs/docs/workflows/troubleshooting.md

Co-authored-by: Thomas Tendyck <51411342+thomasten@users.noreply.github.com>

* docs: elaborate on how to get boot logs

* bazel: tidy

---------

Co-authored-by: Daniel Weiße <66256922+daniel-weisse@users.noreply.github.com>
Co-authored-by: Thomas Tendyck <51411342+thomasten@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Change breaks existing API or configuration.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants