Skip to content

feat: embedded docs#397

Draft
ralgozino wants to merge 2 commits intomainfrom
feat/embedded-docs
Draft

feat: embedded docs#397
ralgozino wants to merge 2 commits intomainfrom
feat/embedded-docs

Conversation

@ralgozino
Copy link
Copy Markdown
Member

@ralgozino ralgozino commented Apr 23, 2025

Summary 💡

  • Add docs pseudo module manifests
  • Add docs pseudo module to on-premises schema and set default values

Relates:

Note

Opening the PR as draft until I add the missing providers after we decided the open points below.

Description 📝

This PR adds a new docs "pseudo-module" (all the manifests live here in the distribution repo, there is no actual docs module). This pseudo-module deploys a special build of the docs site, with the documentation for only the version of SD that you are installing.

This version of the docs site is build in our docs repository and has working local search.

The docs site will be accessible at an automatically created ingress docs.<base domain>. The ingress will not be under SSO even if enabled, documentation is publicly accessible anyway.

There are some overrides that the user can perform, as per usual, like the ingress host, ingress class and nodeSelector and Tolerations.

Open to discussion:

  1. is it a docs pseudo-module the right place for this? Maybe we could have a utils module (as we have discussed in the past) and this be a part of it?
  2. if enable the docs by default or not, it could be a problem for our testing phases because the site container images won't be available until after we do the release. So we will need to have that into consideration. We can always disable the docs in the e2e or use a patch to change the image tag to an existing one.

You can see a version of the site with the following command:

docker run -p 8080:8080 --rm registry.sighup.io/fury/docs:1.30.2-single

then navigate to localhost:8080

Breaking Changes 💔

None

Tests performed 🧪

Note

All tests have been performed on an on-premises cluster running SD 1.31.1.

  • Enabled / Disabled the docs and verified that the resources are created / deleted.
  • Tested with single ingress
  • Tested with dual ingress
  • Tested with ingress disabled (none):the Ingress resource is not created and the rest of the resources are deployed as expected.
  • Tested overriding the ingress host
  • Tested common tolerations and selectors are properly applied.

Future work 🔧

- Add docs pseudo module manifests
- Add docs pseudo module to on-premises schema and set default values
@ralgozino ralgozino requested a review from nutellinoit April 24, 2025 10:37
@ralgozino ralgozino self-assigned this Apr 24, 2025
@nutellinoit
Copy link
Copy Markdown
Member

  1. is it a docs pseudo-module the right place for this? Maybe we could have a utils module (as we have discussed in the past) and this be a part of it?

I think that maybe this is the right time to start implementing the utils module, and use the "real" module (utils) to enable the docs.

This requires:

  • some development on furyctl to consider also utils module download
  • the new utils module (obviously)
  1. if enable the docs by default or not, it could be a problem for our testing phases because the site container images won't be available until after we do the release. So we will need to have that into consideration. We can always disable the docs in the e2e or use a patch to change the image tag to an existing one.

I think that by default the docs should be disabled, or at least with the ingress disabled. Since it's used by admins, I think that for them doing a port forward to read the docs can be enough (we could also add a tip on the furyctl output after the installation). I'm not against depolying everything by default, but maybe as a "production" setting, having them not accessible could be a good idea (to reduce the surface on the intelligence that can be gathered by external actors).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants