Skip to content

Commit

Permalink
Fix entity too large when using conda file > 10k (#658)
Browse files Browse the repository at this point in the history

Signed-off-by: munishchouhan <hrma017@gmail.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Co-authored-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
  • Loading branch information
munishchouhan and pditommaso authored Oct 2, 2024
1 parent c77d49d commit afeaf83
Show file tree
Hide file tree
Showing 10 changed files with 120 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,12 @@ class BuildConfig {
@Value('${wave.build.retry-attempts:0}')
int retryAttempts

@Value('${wave.build.max-conda-file-size:50000}')
int maxCondaFileSize

@Value('${wave.build.max-container-file-size:10000}')
int maxContainerFileSize

@PostConstruct
private void init() {
log.info("Builder config: " +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,8 @@ class ContainerController {

protected void storeContainerRequest0(SubmitContainerTokenRequest req, ContainerRequestData data, TokenData token, String target, String ip) {
try {
final recrd = new WaveContainerRecord(req, data, target, ip, token.expiration)
persistenceService.saveContainerRequest(token.value, recrd)
final recrd = new WaveContainerRecord(req, data, token.value, target, ip, token.expiration)
persistenceService.saveContainerRequest(recrd)
}
catch (Throwable e) {
log.error("Unable to store container request with token: ${token}", e)
Expand Down Expand Up @@ -522,6 +522,12 @@ class ContainerController {

void validateContainerRequest(SubmitContainerTokenRequest req) throws BadRequestException {
String msg
//check conda file size
if( req.condaFile && req.condaFile.length() > buildConfig.maxCondaFileSize )
throw new BadRequestException("Conda file size exceeds the maximum allowed size of ${buildConfig.maxCondaFileSize} bytes")
// check container file size
if( req.containerFile && req.containerFile.length() > buildConfig.maxContainerFileSize )
throw new BadRequestException("Container file size exceeds the maximum allowed size of ${buildConfig.maxContainerFileSize} bytes")
// check valid image name
msg = validationService.checkContainerName(req.containerImage)
if( msg ) throw new BadRequestException(msg)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,9 @@ interface PersistenceService {
/**
* Store a {@link WaveContainerRecord} object in the Surreal wave_request table.
*
* @param token The request token associated with this request
* @param data A {@link WaveContainerRecord} object representing a Wave request record
*/
void saveContainerRequest(String token, WaveContainerRecord data)
void saveContainerRequest(WaveContainerRecord data)

/**
* Update a container request with the digest of the resolved request
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import groovy.transform.CompileStatic
import groovy.transform.ToString
import groovy.util.logging.Slf4j
import io.seqera.wave.api.ContainerConfig
import io.seqera.wave.api.FusionVersion
import io.seqera.wave.api.SubmitContainerTokenRequest
import io.seqera.wave.service.ContainerRequestData
import io.seqera.wave.tower.User
Expand All @@ -42,6 +41,12 @@ import static io.seqera.wave.util.DataTimeUtils.parseOffsetDateTime
@CompileStatic
class WaveContainerRecord {

/**
* wave request id, this will be the token
* This is container token and it is named as id for surrealdb requirement
*/
final String id

/**
* The Tower user associated with the request
*/
Expand Down Expand Up @@ -158,7 +163,8 @@ class WaveContainerRecord {
*/
final String fusionVersion

WaveContainerRecord(SubmitContainerTokenRequest request, ContainerRequestData data, String waveImage, String addr, Instant expiration) {
WaveContainerRecord(SubmitContainerTokenRequest request, ContainerRequestData data, String token, String waveImage, String addr, Instant expiration) {
this.id = token
this.user = data.identity.user
this.workspaceId = request.towerWorkspaceId
this.containerImage = request.containerImage
Expand All @@ -184,6 +190,7 @@ class WaveContainerRecord {
}

WaveContainerRecord(WaveContainerRecord that, String sourceDigest, String waveDigest) {
this.id = that.id
this.user = that.user
this.workspaceId = that.workspaceId
this.containerImage = that.containerImage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ class LocalPersistenceService implements PersistenceService {
}

@Override
void saveContainerRequest(String token, WaveContainerRecord data) {
requestStore.put(token, data)
void saveContainerRequest(WaveContainerRecord data) {
requestStore.put(data.id, data)
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,12 @@ import io.micronaut.http.annotation.Body
import io.micronaut.http.annotation.Get
import io.micronaut.http.annotation.Header
import io.micronaut.http.annotation.Post
import io.micronaut.http.annotation.Put
import io.micronaut.http.client.annotation.Client
import io.micronaut.retry.annotation.Retryable
import io.seqera.wave.service.mirror.MirrorEntry
import io.seqera.wave.service.persistence.WaveBuildRecord
import io.seqera.wave.service.persistence.WaveScanRecord
import io.seqera.wave.service.scan.ScanVulnerability
import io.seqera.wave.service.persistence.WaveBuildRecord
import io.seqera.wave.service.persistence.WaveContainerRecord
import reactor.core.publisher.Flux
/**
* Declarative http client for SurrealDB
Expand Down Expand Up @@ -74,12 +72,6 @@ interface SurrealClient {
@Get('/key/wave_request/{token}')
String getContainerRequest(@Header String authorization, String token)

@Post('/key/wave_request/{token}')
Flux<Map<String, Object>> insertContainerRequestAsync(@Header String authorization, String token, @Body WaveContainerRecord body)

@Put('/key/wave_request/{token}')
Flux<Map<String, Object>> updateContainerRequestAsync(@Header String authorization, String token, @Body WaveContainerRecord body)

@Post('/key/wave_scan')
Map<String,Object> insertScanRecord(@Header String authorization, @Body WaveScanRecord body)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,21 @@ class SurrealPersistenceService implements PersistenceService {

@Override
void saveBuild(WaveBuildRecord build) {
surrealDb.insertBuildAsync(getAuthorization(), build).subscribe({ result->
log.trace "Build request with id '$build.buildId' saved record: ${result}"
}, {error->
def msg = error.message
if( error instanceof HttpClientResponseException ){
msg += ":\n $error.response.body"
}
log.error("Error saving Build request record ${msg}\n${build}", error)
})
// note: use surreal sql in order to by-pass issue with large payload
// see https://github.com/seqeralabs/wave/issues/559#issuecomment-2369412170
final query = "INSERT INTO wave_build ${JacksonHelper.toJson(build)}"
surrealDb
.sqlAsync(getAuthorization(), query)
.subscribe({result ->
log.trace "Build request with id '$build.buildId' saved record: ${result}"
},
{error->
def msg = error.message
if( error instanceof HttpClientResponseException ){
msg += ":\n $error.response.body"
}
log.error("Error saving Build request record ${msg}\n${build}", error)
})
}

@Override
Expand Down Expand Up @@ -167,16 +173,22 @@ class SurrealPersistenceService implements PersistenceService {
}

@Override
void saveContainerRequest(String token, WaveContainerRecord data) {
surrealDb.insertContainerRequestAsync(authorization, token, data).subscribe({ result->
log.trace "Container request with token '$token' saved record: ${result}"
}, {error->
def msg = error.message
if( error instanceof HttpClientResponseException ){
msg += ":\n $error.response.body"
}
log.error("Error saving container request record ${msg}\n${data}", error)
})
void saveContainerRequest(WaveContainerRecord data) {
// note: use surreal sql in order to by-pass issue with large payload
// see https://github.com/seqeralabs/wave/issues/559#issuecomment-2369412170
final query = "INSERT INTO wave_request ${JacksonHelper.toJson(data)}"
surrealDb
.sqlAsync(getAuthorization(), query)
.subscribe({result ->
log.trace "Container request with token '$data.id' saved record: ${result}"
},
{error->
def msg = error.message
if( error instanceof HttpClientResponseException ){
msg += ":\n $error.response.body"
}
log.error("Error saving container request record ${msg}\n${data}", error)
})
}

void updateContainerRequest(String token, ContainerDigestPair digest) {
Expand Down Expand Up @@ -206,11 +218,15 @@ class SurrealPersistenceService implements PersistenceService {
final json = surrealDb.getContainerRequest(getAuthorization(), token)
log.trace "Container request with token '$token' loaded: ${json}"
final type = new TypeReference<ArrayList<SurrealResult<WaveContainerRecord>>>() {}
final data= json ? JacksonHelper.fromJson(json, type) : null
final data= json ? JacksonHelper.fromJson(patchSurrealId(json,"wave_request"), type) : null
final result = data && data[0].result ? data[0].result[0] : null
return result
}

static protected String patchSurrealId(String json, String table) {
json.replaceFirst(/"id":\s*"${table}:(\w*)"/) { List<String> it-> /"id":"${it[1]}"/ }
}

void createScanRecord(WaveScanRecord scanRecord) {
final result = surrealDb.insertScanRecord(authorization, scanRecord)
log.trace "Scan create result=$result"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,11 @@ class ViewControllerTest extends Specification {

and:
def exp = Instant.now().plusSeconds(3600)
def container = new WaveContainerRecord(req, data, wave, addr, exp)
def token = '12345'
def container = new WaveContainerRecord(req, data, token, wave, addr, exp)

when:
persistenceService.saveContainerRequest(token, container)
persistenceService.saveContainerRequest(container)
and:
def request = HttpRequest.GET("/view/containers/${token}")
def response = client.toBlocking().exchange(request, String)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,10 @@ class WaveContainerRecordTest extends Specification {

when:
def exp = Instant.now().plusSeconds(3600)
def container = new WaveContainerRecord(req, data, wave, addr, exp)
def token = '1234'
def container = new WaveContainerRecord(req, data, token, wave, addr, exp)
then:
container.id == token
container.user == user
container.workspaceId == req.towerWorkspaceId
container.containerImage == req.containerImage
Expand Down Expand Up @@ -101,8 +103,10 @@ class WaveContainerRecordTest extends Specification {

when:
def exp = Instant.now().plusSeconds(3600)
def container = new WaveContainerRecord(req, data, wave, addr, exp)
def token = '1234'
def container = new WaveContainerRecord(req, data, token, wave, addr, exp)
then:
container.id == token
container.user == user
container.workspaceId == req.towerWorkspaceId
container.containerImage == req.containerImage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ import io.seqera.wave.service.scan.ScanVulnerability
import io.seqera.wave.test.SurrealDBTestContainer
import io.seqera.wave.tower.PlatformId
import io.seqera.wave.tower.User
import org.apache.commons.lang3.RandomStringUtils

/**
* @author : jorge <jorge.aguilera@seqera.io>
*
Expand Down Expand Up @@ -233,6 +235,7 @@ class SurrealPersistenceServiceTest extends Specification implements SurrealDBTe

def 'should load a request record' () {
given:
def largeContainerFile = RandomStringUtils.random(25600, true, true)
def persistence = applicationContext.getBean(SurrealPersistenceService)
and:
def TOKEN = '123abc'
Expand All @@ -249,23 +252,23 @@ class SurrealPersistenceServiceTest extends Specification implements SurrealDBTe
timestamp: Instant.now().toString()
)
def user = new User(id: 1, userName: 'foo', email: 'foo@gmail.com')
def data = new ContainerRequestData(new PlatformId(user,100), 'hello-world' )
def data = new ContainerRequestData(new PlatformId(user,100), 'hello-world', largeContainerFile )
def wave = "wave.io/wt/$TOKEN/hello-world"
def addr = "100.200.300.400"
def exp = Instant.now().plusSeconds(3600)
and:
def request = new WaveContainerRecord(req, data, wave, addr, exp)
def request = new WaveContainerRecord(req, data, TOKEN, wave, addr, exp)

and:
persistence.saveContainerRequest(TOKEN, request)
persistence.saveContainerRequest(request)
and:
sleep 200 // <-- the above request is async, give time to save it

when:
def loaded = persistence.loadContainerRequest(TOKEN)
then:
loaded == request

loaded.containerFile == largeContainerFile

// should update the record
when:
Expand Down Expand Up @@ -366,5 +369,47 @@ class SurrealPersistenceServiceTest extends Specification implements SurrealDBTe
stored == result
}

def 'should remove surreal table from json' () {
given:
def json = /{"id":"wave_request:1234abc", "this":"one", "that":123 }/
expect:
SurrealPersistenceService.patchSurrealId(json, "wave_request")
== /{"id":"1234abc", "this":"one", "that":123 }/
}

def 'should save 50KB container and conda file' (){
given:
def data = RandomStringUtils.random(25600, true, true)
def persistence = applicationContext.getBean(SurrealPersistenceService)
final request = new BuildRequest(
'container1234',
data,
data,
Path.of("/some/path"),
'buildrepo:recipe-container1234',
PlatformId.NULL,
ContainerPlatform.of('amd64'),
'docker.io/my/cache',
'127.0.0.1',
'{"config":"json"}',
null,
null,
'scan12345',
null,
BuildFormat.DOCKER,
Duration.ofMinutes(1)
).withBuildId('123')
and:
def result = BuildResult.completed(request.buildId, 1, 'Hello', Instant.now().minusSeconds(60), 'xyz')

and:
def build1 = WaveBuildRecord.fromEvent(new BuildEvent(request, result))

when:
persistence.saveBuild(build1)
sleep 100
then:
persistence.loadBuild(request.buildId) == build1
}

}

0 comments on commit afeaf83

Please sign in to comment.