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

EY-3454: Unngår bruk av kafka-avro-serializer-biblioteket der det ikkje trengs #3666

Merged
merged 5 commits into from
Feb 7, 2024

Conversation

madsop-nav
Copy link
Contributor

Dette biblioteket trekkjer med seg ein eldre versjon av snappy-java, som dukkar opp i fleng på https://console.nav.cloud.nais.io/team/etterlatte/vulnerabilities , og også ein confluent-spesifikk versjon av kafka-clients som eg ikkje har funne koden til.

Unngår dermed å bruke dette biblioteket der det ikkje trengs, trade-offen er eit par ekstra konstantar i kafka-repoet si Avrokonstanter-fil, og det tenkjer eg er vel verdt det.

…vhengnaden dreg med seg mykje greier transitivt
…-appen, denne avhengnaden dreg med seg mykje greier transitivt
…appen, denne avhengnaden dreg med seg mykje greier transitivt
…appen, denne avhengnaden dreg med seg mykje greier transitivt
… det ikkje like rett fram å få bort avhengnaden, men hjelper å minimere også
@madsop-nav madsop-nav requested a review from a team as a code owner February 5, 2024 13:56
Comment on lines 4 to +11
const val SPECIFIC_AVRO_READER_CONFIG = "specific.avro.reader"
const val BASIC_AUTH_CREDENTIALS_SOURCE = SchemaRegistryClientConfig.BASIC_AUTH_CREDENTIALS_SOURCE
const val USER_INFO_CONFIG = SchemaRegistryClientConfig.USER_INFO_CONFIG
const val SCHEMA_REGISTRY_URL_CONFIG = "schema.registry.url"

private object SchemaRegistryClientConfig {
const val BASIC_AUTH_CREDENTIALS_SOURCE = "basic.auth.credentials.source"
const val USER_INFO_CONFIG = "basic.auth.user.info"
Copy link
Contributor

Choose a reason for hiding this comment

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

Bytte ut strings med de statiske verdiene til AbstractKafkaSchemaSerDeConfig?

Og kanskje enda bedre, droppe filen Avrokonstanter og heller bruke det direkte, siden hele denne er en litt unødvendig abstraksjon av AbstractKafkaSchemaSerDeConfig 🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dette er ei avveging, ja. Grunngjevinga var meint å komme fram i PR-tittel pluss første kommentaren over, og ein litt meir utførleg diskusjon hadde vi på #3258

@madsop-nav
Copy link
Contributor Author

madsop-nav commented Feb 6, 2024

Tok ein gjennomgang av sårbarheiter som rapportert inn i Nais-konsollet:

  • etterlatte-institusjonsopphold: alle innslag kjem pga kafka-avro-serializer
  • etterlatte-klage: alle innslag kjem pga kafka-avro-serializer
  • egne-ansatte-lytter: 7 av 9 innslag kjem pga denne
  • hendelser-joark: 7 av 9 kjem pga denne
  • hendelser-pdl: 7 av 9 kjem pga denne
  • hendelser-samordning: 7 av 9 kjem pga denne
  • utbetaling: 0 av 6

Dette er dei einaste appane i dette repoet kor det er sårbarheiter.

Appane i felles:

@madsop-nav madsop-nav changed the title Unngår bruk av kafka-avro-serializer-biblioteket der det ikkje trengs EY-3454: Unngår bruk av kafka-avro-serializer-biblioteket der det ikkje trengs Feb 6, 2024
@madsop-nav madsop-nav merged commit 8937e40 into main Feb 7, 2024
@madsop-nav madsop-nav deleted the vulnerabilities branch February 7, 2024 11:40
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