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

feat(server): support website custom domain ssl cert auto-gen #956

Merged
merged 3 commits into from
Mar 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions deploy/build/charts/laf-server/templates/cert-issuer.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
apiVersion: cert-manager.io/v1
kind: ClusterIssuer
metadata:
name: laf-issuer
spec:
acme:
server: https://acme-v02.api.letsencrypt.org/directory
email: admin@sealos.io
privateKeySecretRef:
name: letsencrypt-prod
solvers:
- http01:
ingress:
class: apisix
Copy link

Choose a reason for hiding this comment

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

code review:

  1. The apiVersion and kind specified in the code patch seem to be correct for a ClusterIssuer.
  2. The metadata section looks correct with a name specified for the issuer.
  3. The acme server URL is the Let's Encrypt production URL which is correct.
  4. The email address specified looks to be valid.
  5. The privateKeySecretRef section looks good, with the name specified correctly.
  6. The solvers section looks good with the http01 solver specified and the ingress class set to apisix.

Overall, the code patch looks good and there are no obvious bugs or risks. As an improvement suggestion, it might be useful to add comments to explain the purpose of each section.

4 changes: 4 additions & 0 deletions server/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ export class ServerConfig {
return process.env.API_SERVER_URL || 'http://localhost:3000'
}

static get certManagerIssuerName() {
return process.env.CERT_MANAGER_ISSUER_NAME || 'laf-issuer'
}

/** default region conf */
static get DEFAULT_REGION_DATABASE_URL() {
return process.env.DEFAULT_REGION_DATABASE_URL
Copy link

Choose a reason for hiding this comment

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

with a brief review of the code patch included:

This code looks correct in terms of syntax, but there are a few points to consider when doing a code review.

  1. Does the code follow the coding standards that have been established for the project?
  2. Are there any potential security risks with the code patch?
  3. Is there any potential for performance issues with the code patch?
  4. Is there any potential for the code patch to introduce any bugs into the existing codebase?
  5. Is the code scalable and maintainable?

It is also important to consider the context in which the code patch is being used. For example, if the code patch is being used in a production environment, it would be important to ensure that all necessary tests have been performed to ensure stability and reliability.

Overall, this code patch looks to be in good condition, but it is always best practice to review code patches carefully before applying them to an existing codebase.

Expand Down
171 changes: 171 additions & 0 deletions server/src/gateway/apisix-custom-cert.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
import { Injectable, Logger } from '@nestjs/common'
import { Region, WebsiteHosting } from '@prisma/client'
import { LABEL_KEY_APP_ID, ServerConfig } from 'src/constants'
import { ClusterService } from 'src/region/cluster/cluster.service'
import { GetApplicationNamespaceByAppId } from 'src/utils/getter'

// This class handles the creation and deletion of website domain certificates
// and ApisixTls resources using Kubernetes Custom Resource Definitions (CRDs).
@Injectable()
export class ApisixCustomCertService {
private readonly logger = new Logger(ApisixCustomCertService.name)
constructor(private readonly clusterService: ClusterService) {}

// Read a certificate for a given website using cert-manager.io CRD
async readWebsiteDomainCert(region: Region, website: WebsiteHosting) {
try {
// Get the namespace based on the application ID
const namespace = GetApplicationNamespaceByAppId(website.appid)
// Create a Kubernetes API client for the specified region
const api = this.clusterService.makeCustomObjectApi(region)

// Make a request to read the Certificate resource
const res = await api.getNamespacedCustomObject(
'cert-manager.io',
'v1',
namespace,
'certificates',
website.id,
)

return res.body
} catch (err) {
if (err?.response?.body?.reason === 'NotFound') return null
this.logger.error(err)
this.logger.error(err?.response?.body)
throw err
}
}

// Create a certificate for a given website using cert-manager.io CRD
async createWebsiteDomainCert(region: Region, website: WebsiteHosting) {
// Get the namespace based on the application ID
const namespace = GetApplicationNamespaceByAppId(website.appid)
// Create a Kubernetes API client for the specified region
const api = this.clusterService.makeObjectApi(region)

// Make a request to create the Certificate resource
const res = await api.create({
apiVersion: 'cert-manager.io/v1',
kind: 'Certificate',
// Set the metadata for the Certificate resource
metadata: {
name: website.id,
namespace,
labels: {
'laf.dev/website': website.id,
'laf.dev/website-domain': website.domain,
[LABEL_KEY_APP_ID]: website.appid,
},
},
// Define the specification for the Certificate resource
spec: {
secretName: website.id,
dnsNames: [website.domain],
issuerRef: {
name: ServerConfig.certManagerIssuerName,
kind: 'ClusterIssuer',
},
},
})
return res.body
}

// Delete a certificate for a given website using cert-manager.io CRD
async deleteWebsiteDomainCert(region: Region, website: WebsiteHosting) {
// Get the namespace based on the application ID
const namespace = GetApplicationNamespaceByAppId(website.appid)
// Create a Kubernetes API client for the specified region
const api = this.clusterService.makeObjectApi(region)

// Make a request to delete the Certificate resource
const res = await api.delete({
apiVersion: 'cert-manager.io/v1',
kind: 'Certificate',
metadata: {
name: website.id,
namespace,
},
})
return res.body
}

// Read an ApisixTls resource for a given website using apisix.apache.org CRD
async readWebsiteDomainApisixTls(region: Region, website: WebsiteHosting) {
try {
// Get the namespace based on the application ID
const namespace = GetApplicationNamespaceByAppId(website.appid)
// Create an API object for the specified region
const api = this.clusterService.makeCustomObjectApi(region)

// Make a request to read the ApisixTls resource
const res = await api.getNamespacedCustomObject(
'apisix.apache.org',
'v2',
namespace,
'apisixtlses',
website.id,
)
return res.body
} catch (err) {
if (err?.response?.body?.reason === 'NotFound') return null
this.logger.error(err)
this.logger.error(err?.response?.body)
throw err
}
}

// Create an ApisixTls resource for a given website using apisix.apache.org CRD
async createWebsiteDomainApisixTls(region: Region, website: WebsiteHosting) {
// Get the namespace based on the application ID
const namespace = GetApplicationNamespaceByAppId(website.appid)
// Create an API object for the specified region
const api = this.clusterService.makeObjectApi(region)

// Make a request to create the ApisixTls resource
const res = await api.create({
apiVersion: 'apisix.apache.org/v2',
kind: 'ApisixTls',
// Set the metadata for the ApisixTls resource
metadata: {
name: website.id,
namespace,
labels: {
'laf.dev/website': website.id,
'laf.dev/website-domain': website.domain,
[LABEL_KEY_APP_ID]: website.appid,
},
},
// Define the specification for the ApisixTls resource
spec: {
hosts: [website.domain],
secret: {
name: website.id,
namespace,
},
},
})
return res.body
}

// Deletes the APISIX TLS configuration for a specific website domain
async deleteWebsiteDomainApisixTls(region: Region, website: WebsiteHosting) {
// Get the application namespace using the website's appid
const namespace = GetApplicationNamespaceByAppId(website.appid)

// Create an API object for the specified region
const api = this.clusterService.makeObjectApi(region)

// Send a delete request to remove the APISIX TLS configuration
const res = await api.delete({
apiVersion: 'apisix.apache.org/v2',
kind: 'ApisixTls',
metadata: {
name: website.id,
namespace,
},
})

return res.body
}
}
25 changes: 25 additions & 0 deletions server/src/gateway/apisix.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export class ApisixService {
const namespace = GetApplicationNamespaceByAppId(appid)
const upstreamNode = `${appid}.${namespace}:8000`

// TODO: use appid as route id instead of `app-{appid}
const id = `app-${appid}`
const data = {
name: id,
Expand Down Expand Up @@ -46,6 +47,7 @@ export class ApisixService {
}

async deleteAppRoute(region: Region, appid: string) {
// TODO: use appid as route id instead of `app-{appid}`
const id = `app-${appid}`
const res = await this.deleteRoute(region, id)
return res
Expand All @@ -57,6 +59,7 @@ export class ApisixService {
const minioUrl = new URL(region.storageConf.internalEndpoint)
const upstreamNode = minioUrl.host

// TODO: use bucket object id as route id instead of bucket name
const id = `bucket-${bucketName}`
const data = {
name: id,
Expand Down Expand Up @@ -88,6 +91,7 @@ export class ApisixService {
}

async deleteBucketRoute(region: Region, bucketName: string) {
// TODO: use bucket object id as route id instead of bucket name
const id = `bucket-${bucketName}`
const res = await this.deleteRoute(region, id)
return res
Expand Down Expand Up @@ -162,6 +166,27 @@ export class ApisixService {
}
}

async getRoute(region: Region, id: string) {
const conf = region.gatewayConf
const api_url = `${conf.apiUrl}/routes/${id}`

try {
const res = await this.httpService.axiosRef.get(api_url, {
headers: {
'X-API-KEY': conf.apiKey,
'Content-Type': 'application/json',
},
})
return res.data
} catch (error) {
if (error?.response?.status === 404) {
return null
}
this.logger.error(error, error.response?.data)
return error
}
}

async deleteRoute(region: Region, id: string) {
const conf = region.gatewayConf
const api_url = `${conf.apiUrl}/routes/${id}`
Copy link

Choose a reason for hiding this comment

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

by looking at the code and seeing if there are any potential bugs:

  1. We should check if the api_url is properly encoded before making an HTTP request. This can be done using encodeURI() or encodeURIComponent()
  2. We should also check if the id parameter is properly sanitized before being used in the api_url. If not, it could lead to security vulnerabilities such as SQL injection.
  3. We should make sure that the response from the API is properly handled and that any errors are handled gracefully.

In terms of improvement suggestions, we can look at refactoring the code to make it more readable and easier to maintain. We could also add additional checks and validations to ensure the data being passed in is valid. Finally, we could add unit tests to ensure that the code is doing what it is intended to do.

Expand Down
2 changes: 2 additions & 0 deletions server/src/gateway/gateway.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { BucketDomainService } from './bucket-domain.service'
import { WebsiteTaskService } from './website-task.service'
import { BucketDomainTaskService } from './bucket-domain-task.service'
import { RuntimeDomainTaskService } from './runtime-domain-task.service'
import { ApisixCustomCertService } from './apisix-custom-cert.service'

@Module({
imports: [HttpModule],
Expand All @@ -16,6 +17,7 @@ import { RuntimeDomainTaskService } from './runtime-domain-task.service'
WebsiteTaskService,
BucketDomainTaskService,
RuntimeDomainTaskService,
ApisixCustomCertService,
],
exports: [RuntimeDomainService, BucketDomainService],
})
Copy link

Choose a reason for hiding this comment

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

the code review

  1. The code patch looks fine and it is properly indented and spaced.
  2. The import statement for ApisixCrdService is properly added above the @module decorator.
  3. The ApisixCrdService is also added in the providers array, so that it can be used in the application.
  4. Finally, the exports array is also updated to include the ApisixCrdService.

Overall, the code patch looks good and there are no bugs or risks. However, it would be better if the code is properly documented with comments.

Expand Down
Loading